Inject tagging operation permissions for CFN

smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/CfnConverter.java

@@ -25,16 +25,14 @@
 import java.util.Set;
 import software.amazon.smithy.aws.cloudformation.schema.CfnConfig;
 import software.amazon.smithy.aws.cloudformation.schema.CfnException;
+import software.amazon.smithy.aws.cloudformation.schema.fromsmithy.mappers.TaggingMapper;
 import software.amazon.smithy.aws.cloudformation.schema.model.Property;
 import software.amazon.smithy.aws.cloudformation.schema.model.ResourceSchema;
-import software.amazon.smithy.aws.cloudformation.schema.model.Tagging;
 import software.amazon.smithy.aws.cloudformation.traits.CfnNameTrait;
 import software.amazon.smithy.aws.cloudformation.traits.CfnResource;
 import software.amazon.smithy.aws.cloudformation.traits.CfnResourceIndex;
 import software.amazon.smithy.aws.cloudformation.traits.CfnResourceTrait;
 import software.amazon.smithy.aws.traits.ServiceTrait;
-import software.amazon.smithy.aws.traits.tagging.AwsTagIndex;
-import software.amazon.smithy.aws.traits.tagging.TaggableTrait;
 import software.amazon.smithy.jsonschema.JsonSchemaConverter;
 import software.amazon.smithy.jsonschema.JsonSchemaMapper;
 import software.amazon.smithy.jsonschema.PropertyNamingStrategy;
@@ -56,7 +54,6 @@
 import software.amazon.smithy.utils.StringUtils;
 
 public final class CfnConverter {
-    private static final String DEFAULT_TAGS_NAME = "Tags";
     private ClassLoader classLoader = CfnConverter.class.getClassLoader();
     private CfnConfig config = new CfnConfig();
     private final List<Smithy2CfnExtension> extensions = new ArrayList<>();
@@ -305,20 +302,6 @@ private ResourceSchema convertResource(ConversionEnvironment environment, Resour
             builder.addDefinition(definitionName, definition.getValue());
         }
 
-        if (resourceShape.hasTrait(TaggableTrait.class)) {
-            AwsTagIndex tagsIndex = AwsTagIndex.of(environment.context.getModel());
-            TaggableTrait trait = resourceShape.expectTrait(TaggableTrait.class);
-            Tagging.Builder tagBuilder = Tagging.builder()
-                    .taggable(true)
-                    .tagOnCreate(tagsIndex.isResourceTagOnCreate(resourceShape.getId()))
-                    .tagProperty("/properties/" + getTagMemberName(resourceShape))
-                    .cloudFormationSystemTags(!trait.getDisableSystemTags())
-                    // Unless tag-on-create is supported, Smithy tagging means
-                    .tagUpdatable(true);
-
-            builder.tagging(tagBuilder.build());
-        }
-
         // Apply all the mappers' after methods.
         ResourceSchema resourceSchema = builder.build();
         for (CfnMapper mapper : environment.mappers) {
@@ -391,47 +374,8 @@ private StructureShape getCfnResourceStructure(Model model, ResourceShape resour
             }
         });
 
-        injectTagsIfNecessary(builder, model, resource, cfnResource);
+        TaggingMapper.injectTagsMember(config, model, resource, builder);
 
         return builder.build();
     }
-
-    private String getTagMemberName(ResourceShape resource) {
-        return resource.getTrait(TaggableTrait.class)
-                .flatMap(TaggableTrait::getProperty)
-                .map(property -> {
-                    if (config.getDisableCapitalizedProperties()) {
-                        return property;
-                    }
-                    return StringUtils.capitalize(property);
-                })
-                .orElse(DEFAULT_TAGS_NAME);
-    }
-
-    private void injectTagsIfNecessary(
-        StructureShape.Builder builder,
-        Model model,
-        ResourceShape resource,
-        CfnResource cfnResource
-    ) {
-        String tagMemberName = getTagMemberName(resource);
-        if (resource.hasTrait(TaggableTrait.class)) {
-            AwsTagIndex tagIndex = AwsTagIndex.of(model);
-            TaggableTrait trait = resource.expectTrait(TaggableTrait.class);
-            if (!trait.getProperty().isPresent() || !cfnResource.getProperties()
-                    .containsKey(trait.getProperty().get())) {
-                if (trait.getProperty().isPresent()) {
-                    ShapeId definition = resource.getProperties().get(trait.getProperty().get());
-                    builder.addMember(tagMemberName, definition);
-                } else {
-                    // A valid TagResource operation certainly has a single tags input member.
-                    AwsTagIndex awsTagIndex = AwsTagIndex.of(model);
-                    Optional<ShapeId> tagOperation = tagIndex.getTagResourceOperation(resource.getId());
-                    MemberShape member = awsTagIndex.getTagsMember(tagOperation.get()).get();
-                    member = member.toBuilder().id(builder.getId().withMember(tagMemberName)).build();
-                    builder.addMember(member);
-                }
-            }
-        }
-    }
 }

smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/CoreExtension.java

@@ -34,6 +34,7 @@ public List<CfnMapper> getCfnMappers() {
                 new IdentifierMapper(),
                 new JsonAddMapper(),
                 new MutabilityMapper(),
-                new RequiredMapper());
+                new RequiredMapper(),
+                new TaggingMapper());
     }
 }

smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/HandlerPermissionMapper.java

@@ -40,7 +40,7 @@
  * @see <a href="https://docs.aws.amazon.com/cloudformation-cli/latest/userguide/resource-type-schema.html#schema-properties-handlers">handlers Docs</a>
  */
 @SmithyInternalApi
-public final class HandlerPermissionMapper implements CfnMapper {
+final class HandlerPermissionMapper implements CfnMapper {
     @Override
     public void before(Context context, ResourceSchema.Builder resourceSchema) {
         if (context.getConfig().getDisableHandlerPermissionGeneration()) {
@@ -97,7 +97,7 @@ public void before(Context context, ResourceSchema.Builder resourceSchema) {
                         .permissions(permissions).build()));
     }
 
-    private Set<String> getPermissionsEntriesForOperation(Model model, ServiceShape service, ShapeId operationId) {
+    static Set<String> getPermissionsEntriesForOperation(Model model, ServiceShape service, ShapeId operationId) {
         OperationShape operation = model.expectShape(operationId, OperationShape.class);
         Set<String> permissionsEntries = new TreeSet<>();
 

smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/TaggingMapper.java

@@ -0,0 +1,113 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package software.amazon.smithy.aws.cloudformation.schema.fromsmithy.mappers;
+
+import static software.amazon.smithy.aws.cloudformation.schema.fromsmithy.mappers.HandlerPermissionMapper.getPermissionsEntriesForOperation;
+
+import java.util.Optional;
+import software.amazon.smithy.aws.cloudformation.schema.CfnConfig;
+import software.amazon.smithy.aws.cloudformation.schema.fromsmithy.CfnMapper;
+import software.amazon.smithy.aws.cloudformation.schema.fromsmithy.Context;
+import software.amazon.smithy.aws.cloudformation.schema.model.ResourceSchema;
+import software.amazon.smithy.aws.cloudformation.schema.model.Tagging;
+import software.amazon.smithy.aws.cloudformation.traits.CfnResource;
+import software.amazon.smithy.aws.cloudformation.traits.CfnResourceIndex;
+import software.amazon.smithy.aws.traits.tagging.AwsTagIndex;
+import software.amazon.smithy.aws.traits.tagging.TaggableTrait;
+import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.shapes.MemberShape;
+import software.amazon.smithy.model.shapes.ResourceShape;
+import software.amazon.smithy.model.shapes.ServiceShape;
+import software.amazon.smithy.model.shapes.ShapeId;
+import software.amazon.smithy.model.shapes.StructureShape;
+import software.amazon.smithy.utils.SmithyInternalApi;
+import software.amazon.smithy.utils.StringUtils;
+
+/**
+ * Generates the resource's Tagging configuration based on the AwsTagIndex, including
+ * the tagging property and operations that interact with tags.
+ *
+ * @see <a href="https://github.com/aws-cloudformation/cloudformation-cli/blob/master/src/rpdk/core/data/schema/provider.definition.schema.v1.json#L198">permissions property definition</a>
+ */
+@SmithyInternalApi
+public final class TaggingMapper implements CfnMapper {
+    private static final String DEFAULT_TAGS_NAME = "Tags";
+
+    @SmithyInternalApi
+    public static void injectTagsMember(
+            CfnConfig config,
+            Model model,
+            ResourceShape resource,
+            StructureShape.Builder builder
+    ) {
+        String tagMemberName = getTagMemberName(config, resource);
+        if (resource.hasTrait(TaggableTrait.class)) {
+            AwsTagIndex tagIndex = AwsTagIndex.of(model);
+            TaggableTrait trait = resource.expectTrait(TaggableTrait.class);
+            CfnResourceIndex resourceIndex = CfnResourceIndex.of(model);
+            CfnResource cfnResource = resourceIndex.getResource(resource).get();
+
+            if (!trait.getProperty().isPresent() || !cfnResource.getProperties()
+                    .containsKey(trait.getProperty().get())) {
+                if (trait.getProperty().isPresent()) {
+                    ShapeId definition = resource.getProperties().get(trait.getProperty().get());
+                    builder.addMember(tagMemberName, definition);
+                } else {
+                    // A valid TagResource operation certainly has a single tags input member.
+                    Optional<ShapeId> tagOperation = tagIndex.getTagResourceOperation(resource.getId());
+                    MemberShape member = tagIndex.getTagsMember(tagOperation.get()).get();
+                    member = member.toBuilder().id(builder.getId().withMember(tagMemberName)).build();
+                    builder.addMember(member);
+                }
+            }
+        }
+    }
+
+    @Override
+    public ResourceSchema after(Context context, ResourceSchema resourceSchema) {
+        ResourceShape resourceShape = context.getResource();
+        if (!resourceShape.hasTrait(TaggableTrait.class)) {
+            return resourceSchema;
+        }
+
+        Model model = context.getModel();
+        ServiceShape service = context.getService();
+        AwsTagIndex tagsIndex = AwsTagIndex.of(model);
+        TaggableTrait trait = resourceShape.expectTrait(TaggableTrait.class);
+        Tagging.Builder tagBuilder = Tagging.builder()
+                .taggable(true)
+                .tagOnCreate(tagsIndex.isResourceTagOnCreate(resourceShape.getId()))
+                .tagProperty("/properties/" + getTagMemberName(context.getConfig(), resourceShape))
+                .cloudFormationSystemTags(!trait.getDisableSystemTags())
+                // Unless tag-on-create is supported, Smithy tagging means
+                .tagUpdatable(true);
+
+        // Add the tagging permissions based on the defined tagging operations.
+        tagsIndex.getTagResourceOperation(resourceShape)
+                .map(operation -> getPermissionsEntriesForOperation(model, service, operation))
+                .ifPresent(tagBuilder::addPermissions);
+        tagsIndex.getListTagsForResourceOperation(resourceShape)
+                .map(operation -> getPermissionsEntriesForOperation(model, service, operation))
+                .ifPresent(tagBuilder::addPermissions);
+        tagsIndex.getUntagResourceOperation(resourceShape)
+                .map(operation -> getPermissionsEntriesForOperation(model, service, operation))
+                .ifPresent(tagBuilder::addPermissions);
+
+        return resourceSchema.toBuilder().tagging(tagBuilder.build()).build();
+    }
+
+    private static String getTagMemberName(CfnConfig config, ResourceShape resource) {
+        return resource.getTrait(TaggableTrait.class)
+                .flatMap(TaggableTrait::getProperty)
+                .map(property -> {
+                    if (config.getDisableCapitalizedProperties()) {
+                        return property;
+                    }
+                    return StringUtils.capitalize(property);
+                })
+                .orElse(DEFAULT_TAGS_NAME);
+    }
+}

smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/model/Tagging.java

@@ -15,6 +15,10 @@
 
 package software.amazon.smithy.aws.cloudformation.schema.model;
 
+import java.util.Collection;
+import java.util.Set;
+import java.util.TreeSet;
+import software.amazon.smithy.utils.SetUtils;
 import software.amazon.smithy.utils.SmithyBuilder;
 import software.amazon.smithy.utils.ToSmithyBuilder;
 
@@ -27,13 +31,15 @@ public final class Tagging implements ToSmithyBuilder<Tagging> {
     private final boolean tagUpdatable;
     private final String tagProperty;
     private final boolean cloudFormationSystemTags;
+    private final Set<String> permissions;
 
     private Tagging(Builder builder) {
         taggable = builder.taggable;
         tagOnCreate = builder.tagOnCreate;
         tagUpdatable = builder.tagUpdatable;
         cloudFormationSystemTags = builder.cloudFormationSystemTags;
         tagProperty = builder.tagProperty;
+        this.permissions = SetUtils.orderedCopyOf(builder.permissions);
     }
 
     public static Builder builder() {
@@ -85,14 +91,24 @@ public String getTagProperty() {
         return tagProperty;
     }
 
+    /**
+     * Returns the set of permissions required to interact with this resource's tags.
+     *
+     * @return the set of permissions.
+     */
+    public Set<String> getPermissions() {
+        return permissions;
+    }
+
     @Override
     public Builder toBuilder() {
         return builder()
                 .taggable(taggable)
                 .tagOnCreate(tagOnCreate)
                 .tagUpdatable(tagUpdatable)
                 .cloudFormationSystemTags(cloudFormationSystemTags)
-                .tagProperty(tagProperty);
+                .tagProperty(tagProperty)
+                .permissions(permissions);
     }
 
     public static final class Builder implements SmithyBuilder<Tagging> {
@@ -101,6 +117,7 @@ public static final class Builder implements SmithyBuilder<Tagging> {
         private boolean tagUpdatable;
         private boolean cloudFormationSystemTags;
         private String tagProperty;
+        private final Set<String> permissions = new TreeSet<>();
 
         @Override
         public Tagging build() {
@@ -131,5 +148,28 @@ public Builder tagProperty(String tagProperty) {
             this.tagProperty = tagProperty;
             return this;
         }
+
+        public Builder permissions(Collection<String> permissions) {
+            this.permissions.clear();
+            this.permissions.addAll(permissions);
+            return this;
+        }
+
+        public Builder addPermissions(Collection<String> permissions) {
+            for (String permission : permissions) {
+                addPermission(permission);
+            }
+            return this;
+        }
+
+        public Builder addPermission(String permission) {
+            this.permissions.add(permission);
+            return this;
+        }
+
+        public Builder clearPermissions() {
+            this.permissions.clear();
+            return this;
+        }
     }
 }

smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/provider.definition.schema.v1.json

@@ -145,6 +145,13 @@
                     "description": "A reference to the Tags property in the schema.",
                     "$ref": "http://json-schema.org/draft-07/schema#/properties/$ref",
                     "default": "/properties/Tags"
+                },
+                "permissions": {
+                    "type": "array",
+                    "items": {
+                        "type": "string"
+                    },
+                    "additionalItems": false
                 }
             },
             "required": [

smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/weather-service-wide.cfn.json

@@ -83,7 +83,12 @@
         "tagProperty": "/properties/Tags",
         "tagUpdatable": true,
         "cloudFormationSystemTags": true,
-        "taggable": true
+        "taggable": true,
+        "permissions": [
+            "weather:ListTagsForResource",
+            "weather:TagResource",
+            "weather:UntagResource"
+        ]
     },
     "additionalProperties": false
 }

smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/weather.cfn.json

@@ -82,7 +82,12 @@
         "tagProperty": "/properties/Tags",
         "tagUpdatable": true,
         "cloudFormationSystemTags": true,
-        "taggable": true
+        "taggable": true,
+        "permissions": [
+            "weather:ListTagsForCity",
+            "weather:TagCity",
+            "weather:UntagCity"
+        ]
     },
     "additionalProperties": false
 }

smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TaggableResourceValidator.java

@@ -43,14 +43,23 @@ public List<ValidationEvent> validate(Model model) {
         List<ValidationEvent> events = new LinkedList<>();
         TopDownIndex topDownIndex = TopDownIndex.of(model);
         AwsTagIndex tagIndex = AwsTagIndex.of(model);
-        for (ServiceShape service : model.getServiceShapesWithTrait(TagEnabledTrait.class)) {
+        for (ServiceShape service : model.getServiceShapes()) {
             for (ResourceShape resource : topDownIndex.getContainedResources(service)) {
+                boolean resourceLikelyTaggable = false;
                 if (resource.hasTrait(TaggableTrait.class)) {
                     events.addAll(validateResource(model, resource, service, tagIndex));
-                } else if (resource.hasTrait(ArnTrait.class) && tagIndex.serviceHasTagApis(service.getId())) {
+                    resourceLikelyTaggable = true;
+                } else if (resource.hasTrait(ArnTrait.class) && tagIndex.serviceHasTagApis(service)) {
                     // If a resource does not have the taggable trait, but has an ARN, and the service has tag
                     // operations, it is most likely a mistake.
                     events.add(warning(resource, "Resource is likely missing `aws.api#taggable` trait."));
+                    resourceLikelyTaggable = true;
+                }
+
+                // It's possible the resource was marked as taggable but the service isn't tagEnabled.
+                if (resourceLikelyTaggable && !service.hasTrait(TagEnabledTrait.class)) {
+                    events.add(warning(service, "Service has resources with `aws.api#taggable` applied but does not "
+                            + "have the `aws.api#tagEnabled` trait."));
                 }
             }
         }

smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/tagging-warnings.errors

@@ -3,3 +3,4 @@
 [WARNING] example.weather#Weather: Service marked `aws.api#TagEnabled` is missing an operation named 'UntagResource.' | ServiceTagging
 [WARNING] example.weather#City: Suggested tag property name is '[T|t]ags'. | TagResourcePropertyName
 [DANGER] example.weather#UpdateCity: Update and put resource lifecycle operations should not support updating tags because it is a privileged operation that modifies access. | TaggableResource
+[WARNING] example.weather#Weather2: Service has resources with `aws.api#taggable` applied but does not have the `aws.api#tagEnabled` trait. | TaggableResource