Fixing some of the contract tests for Domain.

aws-codeartifact-domain/aws-codeartifact-domain.json

@@ -6,7 +6,7 @@
     "DomainName": {
       "description": "The name of the domain.",
       "type": "string",
-      "pattern": "[a-z][a-z0-9\\-]{0,48}[a-z0-9]",
+      "pattern": "^([a-z][a-z0-9\\-]{0,48}[a-z0-9])$",
       "minLength": 2,
       "maxLength": 50
     },
@@ -39,12 +39,9 @@
       ],
       "type": "string"
     },
-    "PolicyDocument": {
+    "PermissionsPolicyDocument": {
       "description": "The access control resource policy on the provided domain.",
-      "type": [
-        "object",
-        "string"
-      ],
+      "type": "object",
       "minLength": 2,
       "maxLength": 5120
     },
@@ -56,6 +53,9 @@
     }
   },
   "additionalProperties": false,
+  "required": [
+    "DomainName"
+  ],
   "createOnlyProperties": [
     "/properties/DomainName",
     "/properties/EncryptionKey"
@@ -64,7 +64,8 @@
     "/properties/Arn",
     "/properties/CreatedTime",
     "/properties/RepositoryCount",
-    "/properties/AssetSizeBytes"
+    "/properties/AssetSizeBytes",
+    "/properties/DomainOwner"
   ],
   "primaryIdentifier": [
     "/properties/Arn"

aws-codeartifact-domain/overrides.json

@@ -0,0 +1,17 @@
+{
+  "CREATE": {
+    "/PermissionsPolicyDocument": {
+      "Version": "2012-10-17",
+      "Statement": [
+        {
+          "Effect": "Allow",
+          "Principal": "*",
+          "Action": "codeartifact:*",
+          "Resource": "*"
+        }
+      ]
+    },
+    "/EncryptionKey": null,
+    "/DomainOwner": null
+  }
+}

aws-codeartifact-domain/src/main/java/software/amazon/codeartifact/domain/BaseHandlerStd.java

@@ -1,22 +1,20 @@
 package software.amazon.codeartifact.domain;
 
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
 import software.amazon.awssdk.awscore.exception.AwsServiceException;
 import software.amazon.awssdk.services.codeartifact.CodeartifactClient;
-import software.amazon.awssdk.services.codeartifact.model.DescribeDomainResponse;
-import software.amazon.awssdk.services.codeartifact.model.GetDomainPermissionsPolicyRequest;
 import software.amazon.awssdk.services.codeartifact.model.PutDomainPermissionsPolicyResponse;
 import software.amazon.awssdk.services.codeartifact.model.ResourceNotFoundException;
 import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy;
 import software.amazon.cloudformation.proxy.Logger;
 import software.amazon.cloudformation.proxy.ProgressEvent;
 import software.amazon.cloudformation.proxy.ProxyClient;
 import software.amazon.cloudformation.proxy.ResourceHandlerRequest;
-import software.amazon.cloudformation.proxy.delay.Constant;
-import org.apache.http.HttpStatus;
-import java.time.Duration;
 
 public abstract class BaseHandlerStd extends BaseHandler<CallbackContext> {
-
+  private static final ObjectMapper MAPPER = new ObjectMapper();
   @Override
   public final ProgressEvent<ResourceModel, CallbackContext> handleRequest(
     final AmazonWebServicesClientProxy proxy,
@@ -47,40 +45,54 @@ protected ProgressEvent<ResourceModel, CallbackContext> putDomainPermissionsPoli
       final ProxyClient<CodeartifactClient> proxyClient,
       final Logger logger
   ) {
-    final ResourceModel desiredModel = progress.getResourceModel();
-    if (desiredModel.getPolicyDocument() == null) {
-      return ProgressEvent.progress(desiredModel, callbackContext);
-    }
-    return proxy.initiate("AWS-CodeArtifact-Domain::Update::PutDomainPermissionsPolicy", proxyClient,
-        progress.getResourceModel(), progress.getCallbackContext())
-        .translateToServiceRequest(Translator::translatePutDomainPolicyRequest)
-        .makeServiceCall((awsRequest, client) -> {
-          PutDomainPermissionsPolicyResponse awsResponse = null;
-          try {
-            awsResponse = client.injectCredentialsAndInvokeV2(awsRequest, client.client()::putDomainPermissionsPolicy);
-            logger.log("Domain permission policy successfully added.");
-          } catch (final AwsServiceException e) {
-            String domainName = desiredModel.getDomainName();
-            Translator.throwCfnException(e, Constants.PUT_DOMAIN_POLICY, domainName);
-          }
-          return awsResponse;
-        })
-        .stabilize((awsRequest, awsResponse, client, model, context) -> domainPolicyIsStabilized(model, client, request))
-        .progress();
+
+      final ResourceModel desiredModel = progress.getResourceModel();
+      final ResourceModel previousModel = request.getPreviousResourceState();
+
+      if (desiredModel.getPermissionsPolicyDocument() == null || policyIsUnchanged(desiredModel, previousModel)) {
+          return ProgressEvent.progress(desiredModel, callbackContext);
+      }
+      return proxy.initiate("AWS-CodeArtifact-Domain::Update::PutDomainPermissionsPolicy", proxyClient, progress.getResourceModel(), progress.getCallbackContext())
+          .translateToServiceRequest(Translator::translatePutDomainPolicyRequest)
+          .makeServiceCall((awsRequest, client) -> {
+              PutDomainPermissionsPolicyResponse awsResponse = null;
+              try {
+                  awsResponse = client.injectCredentialsAndInvokeV2(awsRequest, client.client()::putDomainPermissionsPolicy);
+                  logger.log("Domain permission policy successfully added.");
+              } catch (final AwsServiceException e) {
+                  String domainName = desiredModel.getDomainName();
+                  Translator.throwCfnException(e, Constants.PUT_DOMAIN_POLICY, domainName);
+              }
+              return awsResponse;
+          })
+          .progress();
   }
 
-    private boolean domainPolicyIsStabilized(
+    boolean policyIsUnchanged(final ResourceModel desiredModel, final ResourceModel previousModel) {
+        if (previousModel == null) {
+            return false;
+        }
+
+        if (desiredModel.getPermissionsPolicyDocument() == null || previousModel.getPermissionsPolicyDocument() == null) {
+            return false;
+        }
+        JsonNode desiredPolicy = MAPPER.valueToTree(desiredModel.getPermissionsPolicyDocument());
+        JsonNode currentPolicy = MAPPER.valueToTree(previousModel.getPermissionsPolicyDocument());
+        return desiredPolicy.equals(currentPolicy);
+    }
+
+    protected boolean doesDomainExist(
         final ResourceModel model,
         final ProxyClient<CodeartifactClient> proxyClient,
         ResourceHandlerRequest<ResourceModel> request
     ) {
-      try {
-          proxyClient.injectCredentialsAndInvokeV2(
-              Translator.translateToGetDomainPermissionPolicy(model, request), proxyClient.client()::getDomainPermissionsPolicy);
-          return true;
-      } catch (ResourceNotFoundException e) {
-          return false;
-      }
+        try {
+            proxyClient.injectCredentialsAndInvokeV2(
+                Translator.translateToReadRequest(model, request), proxyClient.client()::describeDomain);
+            return true;
+        } catch (ResourceNotFoundException e) {
+            return false;
+        }
     }
 
 }

aws-codeartifact-domain/src/main/java/software/amazon/codeartifact/domain/Constants.java

@@ -2,12 +2,12 @@
 
 public class Constants {
 
-    public static final String CREATE_DOMAIN = "codeartifact::CreateDomain";
-    public static final String LIST_DOMAINS = "codeartifact::ListDomains";
-    public static final String PUT_DOMAIN_POLICY = "codeartifact::PutDomainPermissionsPolicy";
-    public static final String DELETE_DOMAIN_PERMISSION_POLICY = "codeartifact::DeleteDomainPermissionsPolicy";
-    public static final String DELETE_DOMAIN = "codeartifact::DeleteDomain";
-    public static final String DESCRIBE_DOMAIN = "codeartifact::DescribeDomain";
+    public static final String CREATE_DOMAIN = "codeartifact:CreateDomain";
+    public static final String LIST_DOMAINS = "codeartifact:ListDomains";
+    public static final String PUT_DOMAIN_POLICY = "codeartifact:PutDomainPermissionsPolicy";
+    public static final String DELETE_DOMAIN_PERMISSION_POLICY = "codeartifact:DeleteDomainPermissionsPolicy";
+    public static final String DELETE_DOMAIN = "codeartifact:DeleteDomain";
+    public static final String DESCRIBE_DOMAIN = "codeartifact:DescribeDomain";
     public static final String ACTIVE_STATUS = "Active";
     public static final int MAX_ITEMS = 1000;
 }

aws-codeartifact-domain/src/main/java/software/amazon/codeartifact/domain/CreateHandler.java

@@ -5,6 +5,8 @@
 import software.amazon.awssdk.services.codeartifact.model.CreateDomainRequest;
 import software.amazon.awssdk.services.codeartifact.model.CreateDomainResponse;
 import software.amazon.awssdk.services.codeartifact.model.DescribeDomainResponse;
+import software.amazon.cloudformation.exceptions.CfnInvalidRequestException;
+import software.amazon.cloudformation.exceptions.CfnNotUpdatableException;
 import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy;
 import software.amazon.cloudformation.proxy.Logger;
 import software.amazon.cloudformation.proxy.ProgressEvent;
@@ -22,6 +24,13 @@ protected ProgressEvent<ResourceModel, CallbackContext> handleRequest(
         final ProxyClient<CodeartifactClient> proxyClient,
         final Logger logger) {
 
+        ResourceModel model = request.getDesiredResourceState();
+
+        // Make sure the user isn't trying to assign values to readOnly properties
+        if (hasReadOnlyProperties(model)) {
+            throw new CfnInvalidRequestException("Attempting to set a ReadOnly Property.");
+        }
+
         this.logger = logger;
         return ProgressEvent.progress(request.getDesiredResourceState(), callbackContext)
             .then(progress -> createDomain(proxy, progress, request, proxyClient))
@@ -59,6 +68,11 @@ private CreateDomainResponse createDomainSdkCall(
         return awsResponse;
     }
 
+    private boolean hasReadOnlyProperties(final ResourceModel model) {
+        return model.getAssetSizeBytes() != null || model.getRepositoryCount() != null
+            || model.getCreatedTime() != null || model.getDomainOwner() != null;
+    }
+
     private boolean isStabilized(
         final ResourceModel model,
         final ProxyClient<CodeartifactClient> proxyClient,

aws-codeartifact-domain/src/main/java/software/amazon/codeartifact/domain/DeleteHandler.java

@@ -2,11 +2,10 @@
 
 import software.amazon.awssdk.awscore.AwsResponse;
 import software.amazon.awssdk.awscore.exception.AwsServiceException;
-import software.amazon.awssdk.core.SdkClient;
 import software.amazon.awssdk.services.codeartifact.CodeartifactClient;
-import software.amazon.awssdk.services.codeartifact.model.DescribeDomainResponse;
-import software.amazon.awssdk.services.codeartifact.model.ResourceNotFoundException;
-import software.amazon.cloudformation.exceptions.CfnGeneralServiceException;
+import software.amazon.awssdk.services.codeartifact.model.ConflictException;
+import software.amazon.cloudformation.exceptions.CfnNotFoundException;
+import software.amazon.cloudformation.exceptions.CfnResourceConflictException;
 import software.amazon.cloudformation.proxy.AmazonWebServicesClientProxy;
 import software.amazon.cloudformation.proxy.Logger;
 import software.amazon.cloudformation.proxy.ProgressEvent;
@@ -24,44 +23,43 @@ protected ProgressEvent<ResourceModel, CallbackContext> handleRequest(
         final Logger logger) {
 
         this.logger = logger;
+        ResourceModel model = request.getDesiredResourceState();
+        // STEP 1.0 [initialize a proxy context]
+        ProgressEvent<ResourceModel, CallbackContext> deleteProgressEvent = proxy
+            .initiate("AWS-CodeArtifact-Domain::Delete", proxyClient, model, callbackContext)
+            // STEP 1.1 [construct a body of a request]
+            .translateToServiceRequest(Translator::translateToDeleteRequest)
+            // STEP 1.2 [make an api call]
+            .makeServiceCall((awsRequest, client) -> {
 
-        return ProgressEvent.progress(request.getDesiredResourceState(), callbackContext)
+                // if domain does not exist, deleteDomain does not throw an exception, so we must do this
+                // to be under the ResourceHandler Contract
+                if (!doesDomainExist(model, proxyClient, request)) {
+                    throw new CfnNotFoundException(model.getDomainName(), ResourceModel.TYPE_NAME);
+                }
 
-            // STEP 1.0 [delete/stabilize progress chain - required for resource deletion]
-            .then(progress ->
-                // STEP 1.0 [initialize a proxy context]
-                proxy.initiate("AWS-CodeArtifact-Domain::Delete", proxyClient, progress.getResourceModel(),
-                    progress.getCallbackContext())
-                    // STEP 1.1 [construct a body of a request]
-                    .translateToServiceRequest(Translator::translateToDeleteRequest)
-                    // STEP 1.2 [make an api call]
-                    .makeServiceCall((awsRequest, client) -> {
-                        AwsResponse awsResponse = null;
-                        try {
-                            awsResponse = client.injectCredentialsAndInvokeV2(awsRequest, proxyClient.client()::deleteDomain);
-                        } catch (final AwsServiceException e) {
-                            String domainName = progress.getResourceModel().getDomainName();
-                            Translator.throwCfnException(e, Constants.DELETE_DOMAIN, domainName);
-                        }
+                AwsResponse awsResponse = null;
+                try {
+                    awsResponse = client.injectCredentialsAndInvokeV2(awsRequest, proxyClient.client()::deleteDomain);
+                } catch (final ConflictException e) {
+                    // this happens if the domain has repositories in the account
+                    throw new CfnResourceConflictException(ResourceModel.TYPE_NAME, model.getDomainName(), e.getMessage(), e);
+                } catch (final AwsServiceException e) {
+                    String domainName = model.getDomainName();
+                    Translator.throwCfnException(e, Constants.DELETE_DOMAIN, domainName);
+                }
 
-                        logger.log(String.format("%s successfully deleted.", ResourceModel.TYPE_NAME));
-                        return awsResponse;
-                    })
-                    // STEP 2.3 [Stabilize to check if the resource got deleted]
-                    .stabilize((deleteDomainRequest, deleteDomainResponse, proxyInvocation, model, context) -> isDeleted(model, proxyInvocation, request))
-                    .success());
-    }
+                logger.log(String.format("%s successfully deleted.", ResourceModel.TYPE_NAME));
+                return awsResponse;
+            })
+            // STEP 2.3 [Stabilize to check if the resource got deleted]
+            .stabilize((deleteDomainRequest, deleteDomainResponse, proxyInvocation, resourceModel, context) -> !doesDomainExist(model, proxyClient, request))
+            .success();
 
-    protected boolean isDeleted(final ResourceModel model,
-        final ProxyClient<CodeartifactClient> proxyClient,
-        ResourceHandlerRequest<ResourceModel> request
-    ) {
-        try {
-            proxyClient.injectCredentialsAndInvokeV2(
-                Translator.translateToReadRequest(model, request), proxyClient.client()::describeDomain);
-            return false;
-        } catch (ResourceNotFoundException e) {
-            return true;
-        }
+
+        // according to the ResourceHandler contract we must not return the model in the response
+        deleteProgressEvent.setResourceModel(null);
+        return deleteProgressEvent;
     }
+
 }

aws-codeartifact-domain/src/main/java/software/amazon/codeartifact/domain/ListHandler.java

@@ -46,9 +46,10 @@ public ProgressEvent<ResourceModel, CallbackContext> handleRequest(
         String nextToken = response.nextToken();
 
         return ProgressEvent.<ResourceModel, CallbackContext>builder()
-            .resourceModels(Translator.translateFromListRequest(response))
+            .resourceModels(Translator.translateFromListRequest(response, request))
             .nextToken(nextToken)
             .status(OperationStatus.SUCCESS)
             .build();
     }
+
 }