From 7c57964a754eb89b1fe4ab00a5dcbf601e8e7cc6 Mon Sep 17 00:00:00 2001 From: Kevin Stich Date: Tue, 27 Aug 2024 11:24:33 -0700 Subject: [PATCH] Fix several bugs in AwsTagIndex This commit fixes several groups of bugs in the AwsTagIndex: 1. The index did not handle nested resource shapes. 2. The index did not handle the put lifecycle operation. 3. The index did not account for the `apiConfig` property of the `@taggable` trait. A full suite of tests was added to account for the index's behavior and validate its previous functionality along with fixes for the above. The `getTagsMember` method was stabilized and made non-static. Minor cleanup was done to other tagging related files. --- .../schema/fromsmithy/CfnConverter.java | 10 +- .../aws/traits/tagging/AwsTagIndex.java | 209 +++++++----- .../tagging/ServiceTaggingValidator.java | 47 ++- .../tagging/TagEnabledServiceValidator.java | 3 +- .../TagResourcePropertyNameValidator.java | 4 +- .../TagResourcePropertyTypeValidator.java | 13 +- .../tagging/TaggableResourceValidator.java | 68 ++-- .../aws/traits/tagging/TaggingShapeUtils.java | 58 +--- .../aws/traits/tagging/AwsTagIndexTest.java | 112 ++++++ .../tagging/tagging-warnings.errors | 2 +- .../tagging/aws-tag-index-test-model.smithy | 321 ++++++++++++++++++ 11 files changed, 633 insertions(+), 214 deletions(-) create mode 100644 smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/tagging/AwsTagIndexTest.java create mode 100644 smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/tagging/aws-tag-index-test-model.smithy diff --git a/smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/CfnConverter.java b/smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/CfnConverter.java index 3042c418471..c6c79e352b7 100644 --- a/smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/CfnConverter.java +++ b/smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/CfnConverter.java @@ -44,7 +44,6 @@ import software.amazon.smithy.model.knowledge.TopDownIndex; import software.amazon.smithy.model.node.ObjectNode; import software.amazon.smithy.model.shapes.MemberShape; -import software.amazon.smithy.model.shapes.OperationShape; import software.amazon.smithy.model.shapes.ResourceShape; import software.amazon.smithy.model.shapes.ServiceShape; import software.amazon.smithy.model.shapes.Shape; @@ -425,11 +424,10 @@ private void injectTagsIfNecessary( ShapeId definition = resource.getProperties().get(trait.getProperty().get()); builder.addMember(tagPropertyName, definition); } else { - // Taggability must be through service-wide TagResource operation - OperationShape tagResourceOp = model.expectShape( - tagIndex.getTagResourceOperation(resource.getId()).get(), OperationShape.class); - // A valid TagResource operation certainly has a single tags input member - MemberShape member = AwsTagIndex.getTagsMember(model, tagResourceOp).get(); + // A valid TagResource operation certainly has a single tags input member. + AwsTagIndex awsTagIndex = AwsTagIndex.of(model); + Optional tagOperation = tagIndex.getTagResourceOperation(resource.getId()); + MemberShape member = awsTagIndex.getTagsMember(tagOperation.get()).get(); member = member.toBuilder().id(builder.getId().withMember(tagPropertyName)).build(); builder.addMember(member); } diff --git a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/AwsTagIndex.java b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/AwsTagIndex.java index e20d3c49995..0ec02cb0190 100644 --- a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/AwsTagIndex.java +++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/AwsTagIndex.java @@ -15,25 +15,22 @@ package software.amazon.smithy.aws.traits.tagging; -import java.util.Collection; import java.util.HashMap; import java.util.HashSet; -import java.util.LinkedList; import java.util.Map; import java.util.Optional; import java.util.Set; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.knowledge.KnowledgeIndex; import software.amazon.smithy.model.knowledge.OperationIndex; -import software.amazon.smithy.model.knowledge.PropertyBindingIndex; import software.amazon.smithy.model.knowledge.TopDownIndex; import software.amazon.smithy.model.shapes.MemberShape; import software.amazon.smithy.model.shapes.OperationShape; 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.model.shapes.ToShapeId; +import software.amazon.smithy.model.traits.NoReplaceTrait; import software.amazon.smithy.utils.SmithyUnstableApi; /** @@ -42,6 +39,7 @@ */ @SmithyUnstableApi public final class AwsTagIndex implements KnowledgeIndex { + private final Map operationTagMembers = new HashMap<>(); private final Set servicesWithAllTagOperations = new HashSet<>(); private final Set resourceIsTagOnCreate = new HashSet<>(); private final Set resourceIsTagOnUpdate = new HashSet<>(); @@ -53,30 +51,33 @@ public final class AwsTagIndex implements KnowledgeIndex { private final Set serviceListTagsOperationIsValid = new HashSet<>(); private AwsTagIndex(Model model) { - PropertyBindingIndex propertyBindingIndex = PropertyBindingIndex.of(model); - OperationIndex operationIndex = OperationIndex.of(model); - + // Compute a service's tagging operations. for (ServiceShape service : model.getServiceShapesWithTrait(TagEnabledTrait.class)) { - for (ShapeId resourceId : service.getResources()) { - ResourceShape resource = model.expectShape(resourceId, ResourceShape.class); - if (resource.hasTrait(TaggableTrait.class) - && resource.expectTrait(TaggableTrait.class).getProperty().isPresent()) { + computeTaggingApis(model, service); + if (serviceTagOperationIsValid.contains(service.getId()) + && serviceUntagOperationIsValid.contains(service.getId()) + && serviceListTagsOperationIsValid.contains(service.getId()) + ) { + servicesWithAllTagOperations.add(service.getId()); + } + } - // Check if tag property is specified on create. - if (TaggingShapeUtils.isTagPropertyInInput(resource.getCreate(), model, resource, - propertyBindingIndex)) { - resourceIsTagOnCreate.add(resourceId); - } - // Check if tag property is specified on update. - if (TaggingShapeUtils.isTagPropertyInInput(resource.getUpdate(), model, resource, - propertyBindingIndex)) { - resourceIsTagOnUpdate.add(resourceId); - } - } + // Compute if a resource meets tag-on-create or tag-on-update criteria. + for (ResourceShape resource : model.getResourceShapesWithTrait(TaggableTrait.class)) { + if (!resource.expectTrait(TaggableTrait.class).getProperty().isPresent()) { + continue; } - //Check if service has the three service-wide tagging operations unbound to any resource. - if (verifyTagApis(model, service, operationIndex)) { - servicesWithAllTagOperations.add(service.getId()); + + computeResourceTagLocations(model, resource); + } + + // Compute any operation's tag input member. + OperationIndex operationIndex = OperationIndex.of(model); + for (OperationShape operation : model.getOperationShapes()) { + for (MemberShape memberShape : operationIndex.getInputMembers(operation).values()) { + if (TaggingShapeUtils.isTagDesiredName(memberShape.getMemberName())) { + operationTagMembers.put(operation.getId(), memberShape); + } } } } @@ -85,6 +86,16 @@ public static AwsTagIndex of(Model model) { return new AwsTagIndex(model); } + /** + * Returns the input member that has a name which matches for tags on a TagResource operation. + * + * @param operation operation object to scan the input members of. + * @return an Optional containing the matching input member if present, otherwise an empty Optional. + */ + public Optional getTagsMember(ToShapeId operation) { + return Optional.ofNullable(operationTagMembers.get(operation.toShapeId())); + } + /** * Checks if the given ShapeID references a resource shape that meets tag on create criteria. * If the given ShapeID does not reference a resource shape, will always return false. @@ -96,8 +107,8 @@ public static AwsTagIndex of(Model model) { * @param resourceId ShapeID of the resource to check. * @return true iff the resourceId references a resource shape that has tag-on create behavior. */ - public boolean isResourceTagOnCreate(ShapeId resourceId) { - return resourceIsTagOnCreate.contains(resourceId); + public boolean isResourceTagOnCreate(ToShapeId resourceId) { + return resourceIsTagOnCreate.contains(resourceId.toShapeId()); } /** @@ -111,8 +122,8 @@ public boolean isResourceTagOnCreate(ShapeId resourceId) { * @param resourceId ShapeID of the resource to check. * @return true iff the resourceId references a resource shape that has tag-on update behavior. */ - public boolean isResourceTagOnUpdate(ShapeId resourceId) { - return resourceIsTagOnUpdate.contains(resourceId); + public boolean isResourceTagOnUpdate(ToShapeId resourceId) { + return resourceIsTagOnUpdate.contains(resourceId.toShapeId()); } /** @@ -122,8 +133,8 @@ public boolean isResourceTagOnUpdate(ShapeId resourceId) { * @param serviceShapeId ShapeID of the shape to check. * @return true iff the serviceShapeId references a service shape that has the necessary tagging APIs. */ - public boolean serviceHasTagApis(ShapeId serviceShapeId) { - return servicesWithAllTagOperations.contains(serviceShapeId); + public boolean serviceHasTagApis(ToShapeId serviceShapeId) { + return servicesWithAllTagOperations.contains(serviceShapeId.toShapeId()); } /** @@ -134,8 +145,8 @@ public boolean serviceHasTagApis(ShapeId serviceShapeId) { * @param serviceOrResourceId ShapeID of the service shape to retrieve the qualifying TagResource operation for. * @return The ShapeID of a qualifying TagResource operation if one is found. Returns an empty optional otherwise. */ - public Optional getTagResourceOperation(ShapeId serviceOrResourceId) { - return Optional.ofNullable(shapeToTagOperation.get(serviceOrResourceId)); + public Optional getTagResourceOperation(ToShapeId serviceOrResourceId) { + return Optional.ofNullable(shapeToTagOperation.get(serviceOrResourceId.toShapeId())); } /** @@ -147,8 +158,8 @@ public Optional getTagResourceOperation(ShapeId serviceOrResourceId) { * @return The ShapeID of a qualifying UntagResource operation if one is found. Returns an empty optional * otherwise. */ - public Optional getUntagResourceOperation(ShapeId serviceOrResourceId) { - return Optional.ofNullable(shapeToUntagOperation.get(serviceOrResourceId)); + public Optional getUntagResourceOperation(ToShapeId serviceOrResourceId) { + return Optional.ofNullable(shapeToUntagOperation.get(serviceOrResourceId.toShapeId())); } /** @@ -161,8 +172,8 @@ public Optional getUntagResourceOperation(ShapeId serviceOrResourceId) * @return The ShapeID of a qualifying ListTagsForResource operation if one is found. Returns an empty optional * otherwise. */ - public Optional getListTagsForResourceOperation(ShapeId serviceOrResourceId) { - return Optional.ofNullable(shapeToListTagsOperation.get(serviceOrResourceId)); + public Optional getListTagsForResourceOperation(ToShapeId serviceOrResourceId) { + return Optional.ofNullable(shapeToListTagsOperation.get(serviceOrResourceId.toShapeId())); } /** @@ -173,8 +184,8 @@ public Optional getListTagsForResourceOperation(ShapeId serviceOrResour * @return True iff the service shape has a service bound operation named 'TagResource' that also satisfies * the criteria for a valid TagResource operation. */ - public boolean serviceHasValidTagResourceOperation(ShapeId serviceId) { - return serviceTagOperationIsValid.contains(serviceId); + public boolean serviceHasValidTagResourceOperation(ToShapeId serviceId) { + return serviceTagOperationIsValid.contains(serviceId.toShapeId()); } /** @@ -185,8 +196,8 @@ public boolean serviceHasValidTagResourceOperation(ShapeId serviceId) { * @return True iff the service shape has a service bound operation named 'UntagResource' that also satisfies * the criteria for a valid UntagResource operation. */ - public boolean serviceHasValidUntagResourceOperation(ShapeId serviceId) { - return serviceUntagOperationIsValid.contains(serviceId); + public boolean serviceHasValidUntagResourceOperation(ToShapeId serviceId) { + return serviceUntagOperationIsValid.contains(serviceId.toShapeId()); } /** @@ -197,78 +208,114 @@ public boolean serviceHasValidUntagResourceOperation(ShapeId serviceId) { * @return True iff the service shape has a service bound operation named 'ListTagsForResource' that also satisfies * the criteria for a valid ListTagsForResource operation. */ - public boolean serviceHasValidListTagsForResourceOperation(ShapeId serviceId) { - return serviceListTagsOperationIsValid.contains(serviceId); + public boolean serviceHasValidListTagsForResourceOperation(ToShapeId serviceId) { + return serviceListTagsOperationIsValid.contains(serviceId.toShapeId()); } - private boolean verifyTagApis(Model model, ServiceShape service, OperationIndex operationIndex) { - boolean hasTagApi = false; - boolean hasUntagApi = false; - boolean hasListTagsApi = false; - Collection serviceResources = new LinkedList<>(); - TopDownIndex topDownIndex = TopDownIndex.of(model); - topDownIndex.getContainedResources(service).forEach(resource -> serviceResources.add(resource.getId())); - + private void computeTaggingApis(Model model, ServiceShape service) { Map operationMap = new HashMap<>(); for (ShapeId operationId : service.getOperations()) { operationMap.put(operationId.getName(), operationId); } + calculateTagApi(model, service, operationMap); + calculateUntagApi(model, service, operationMap); + calculateListTagsApi(model, service, operationMap); + } + + private void calculateTagApi( + Model model, + ServiceShape service, + Map operationMap + ) { + TopDownIndex topDownIndex = TopDownIndex.of(model); + OperationIndex operationIndex = OperationIndex.of(model); + if (operationMap.containsKey(TaggingShapeUtils.TAG_RESOURCE_OPNAME)) { ShapeId tagOperationId = operationMap.get(TaggingShapeUtils.TAG_RESOURCE_OPNAME); shapeToTagOperation.put(service.getId(), tagOperationId); - serviceResources.forEach(resourceId -> shapeToTagOperation.put(resourceId, tagOperationId)); OperationShape tagOperation = model.expectShape(tagOperationId, OperationShape.class); - hasTagApi = TaggingShapeUtils.verifyTagResourceOperation(model, service, tagOperation, operationIndex); - if (hasTagApi) { + if (TaggingShapeUtils.verifyTagResourceOperation(model, tagOperation, operationIndex)) { serviceTagOperationIsValid.add(service.getId()); } } + for (ResourceShape resourceShape : topDownIndex.getContainedResources(service)) { + shapeToTagOperation.put(resourceShape.getId(), + resourceShape.getTrait(TaggableTrait.class) + .flatMap(TaggableTrait::getApiConfig) + .map(TaggableApiConfig::getTagApi) + .orElse(shapeToTagOperation.get(service.getId()))); + } + } + + private void calculateUntagApi( + Model model, + ServiceShape service, + Map operationMap + ) { + TopDownIndex topDownIndex = TopDownIndex.of(model); + OperationIndex operationIndex = OperationIndex.of(model); if (operationMap.containsKey(TaggingShapeUtils.UNTAG_RESOURCE_OPNAME)) { ShapeId untagOperationId = operationMap.get(TaggingShapeUtils.UNTAG_RESOURCE_OPNAME); shapeToUntagOperation.put(service.getId(), untagOperationId); - serviceResources.forEach(resourceId -> shapeToUntagOperation.put(resourceId, untagOperationId)); OperationShape untagOperation = model.expectShape(untagOperationId, OperationShape.class); - hasUntagApi = TaggingShapeUtils.verifyUntagResourceOperation(model, service, untagOperation, - operationIndex); - if (hasUntagApi) { + if (TaggingShapeUtils.verifyUntagResourceOperation(model, untagOperation, operationIndex)) { serviceUntagOperationIsValid.add(service.getId()); } } + for (ResourceShape resourceShape : topDownIndex.getContainedResources(service)) { + shapeToUntagOperation.put(resourceShape.getId(), + resourceShape.getTrait(TaggableTrait.class) + .flatMap(TaggableTrait::getApiConfig) + .map(TaggableApiConfig::getUntagApi) + .orElse(shapeToUntagOperation.get(service.getId()))); + } + } + + private void calculateListTagsApi( + Model model, + ServiceShape service, + Map operationMap + ) { + TopDownIndex topDownIndex = TopDownIndex.of(model); + OperationIndex operationIndex = OperationIndex.of(model); if (operationMap.containsKey(TaggingShapeUtils.LIST_TAGS_OPNAME)) { ShapeId listTagsOperationId = operationMap.get(TaggingShapeUtils.LIST_TAGS_OPNAME); shapeToListTagsOperation.put(service.getId(), listTagsOperationId); - serviceResources.forEach(resourceId -> shapeToListTagsOperation.put(resourceId, listTagsOperationId)); OperationShape listTagsOperation = model.expectShape(listTagsOperationId, OperationShape.class); - hasListTagsApi = TaggingShapeUtils.verifyListTagsOperation(model, service, listTagsOperation, - operationIndex); - if (hasListTagsApi) { + if (TaggingShapeUtils.verifyListTagsOperation(model, listTagsOperation, operationIndex)) { serviceListTagsOperationIsValid.add(service.getId()); } } - - return hasTagApi && hasUntagApi && hasListTagsApi; + for (ResourceShape resourceShape : topDownIndex.getContainedResources(service)) { + shapeToListTagsOperation.put(resourceShape.getId(), + resourceShape.getTrait(TaggableTrait.class) + .flatMap(TaggableTrait::getApiConfig) + .map(TaggableApiConfig::getListTagsApi) + .orElse(shapeToListTagsOperation.get(service.getId()))); + } } - /** - * Returns the input member that has a name which matches for tags on a TagResource operation. - * - * Note: This is more of a utility method needed to expose limited access to an internal method. - * - * @param model Smithy model to follow member references from the provided operation shape. - * @param tagResourceOperation TagResource operation object to scan the input members of. - * @return the matching input member if present. {@see java.util.Optional.empty()} otherwise. - */ - @SmithyInternalApi - public static Optional getTagsMember(Model model, OperationShape tagResourceOperation) { - for (MemberShape memberShape : model.expectShape(tagResourceOperation.getInputShape(), - StructureShape.class).members()) { - if (TaggingShapeUtils.isTagDesiredName(memberShape.getMemberName())) { - return Optional.of(memberShape); + private void computeResourceTagLocations(Model model, ResourceShape resource) { + // Check if tag property is specified on create. + if (TaggingShapeUtils.isTagPropertyInInput(resource.getCreate(), model, resource)) { + resourceIsTagOnCreate.add(resource.getId()); + } + // Check if tag property is specified on update. + if (TaggingShapeUtils.isTagPropertyInInput(resource.getUpdate(), model, resource)) { + resourceIsTagOnUpdate.add(resource.getId()); + } + + // Check if tag property is specified on put. + if (TaggingShapeUtils.isTagPropertyInInput(resource.getPut(), model, resource)) { + // Put is always a create operation + resourceIsTagOnCreate.add(resource.getId()); + // and it's also an update when not annotated with `@noReplace`. + if (!resource.hasTrait(NoReplaceTrait.class)) { + resourceIsTagOnUpdate.add(resource.getId()); } } - return Optional.empty(); } } diff --git a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/ServiceTaggingValidator.java b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/ServiceTaggingValidator.java index 80c6a4e39da..9d08b737361 100644 --- a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/ServiceTaggingValidator.java +++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/ServiceTaggingValidator.java @@ -15,11 +15,15 @@ package software.amazon.smithy.aws.traits.tagging; +import static software.amazon.smithy.aws.traits.tagging.TaggingShapeUtils.LIST_TAGS_OPNAME; +import static software.amazon.smithy.aws.traits.tagging.TaggingShapeUtils.TAG_RESOURCE_OPNAME; +import static software.amazon.smithy.aws.traits.tagging.TaggingShapeUtils.UNTAG_RESOURCE_OPNAME; + import java.util.LinkedList; import java.util.List; import java.util.Optional; +import software.amazon.smithy.model.FromSourceLocation; import software.amazon.smithy.model.Model; -import software.amazon.smithy.model.SourceLocation; import software.amazon.smithy.model.shapes.ServiceShape; import software.amazon.smithy.model.shapes.ShapeId; import software.amazon.smithy.model.validation.AbstractValidator; @@ -29,69 +33,58 @@ * Validates service satisfies AWS tagging requirements. */ public final class ServiceTaggingValidator extends AbstractValidator { - private static final String TAG_RESOURCE_OPNAME = "TagResource"; - private static final String UNTAG_RESOURCE_OPNAME = "UntagResource"; - private static final String LISTTAGS_OPNAME = "ListTagsForResource"; - @Override public List validate(Model model) { AwsTagIndex awsTagIndex = AwsTagIndex.of(model); List events = new LinkedList<>(); for (ServiceShape service : model.getServiceShapesWithTrait(TagEnabledTrait.class)) { - events.addAll(validateService(model, service, awsTagIndex)); + events.addAll(validateService(service, awsTagIndex)); } return events; } - private List validateService(Model model, ServiceShape service, AwsTagIndex awsTagIndex) { + private List validateService(ServiceShape service, AwsTagIndex awsTagIndex) { List events = new LinkedList<>(); - SourceLocation tagEnabledTraitLoc = service.expectTrait(TagEnabledTrait.class).getSourceLocation(); + TagEnabledTrait trait = service.expectTrait(TagEnabledTrait.class); Optional tagResourceId = awsTagIndex.getTagResourceOperation(service.getId()); if (tagResourceId.isPresent()) { if (!awsTagIndex.serviceHasValidTagResourceOperation(service.getId())) { - events.add(getMessageUnqualifedOperation(service, tagEnabledTraitLoc, - tagResourceId.get(), TAG_RESOURCE_OPNAME)); + events.add(getInvalidOperationEvent(service, trait, tagResourceId.get(), TAG_RESOURCE_OPNAME)); } } else { - events.add(getMessageMissingOperation(service, tagEnabledTraitLoc, TAG_RESOURCE_OPNAME)); + events.add(getMissingOperationEvent(service, trait, TAG_RESOURCE_OPNAME)); } Optional untagResourceId = awsTagIndex.getUntagResourceOperation(service.getId()); if (untagResourceId.isPresent()) { if (!awsTagIndex.serviceHasValidUntagResourceOperation(service.getId())) { - events.add(getMessageUnqualifedOperation(service, tagEnabledTraitLoc, - untagResourceId.get(), UNTAG_RESOURCE_OPNAME)); + events.add(getInvalidOperationEvent(service, trait, untagResourceId.get(), UNTAG_RESOURCE_OPNAME)); } } else { - events.add(getMessageMissingOperation(service, tagEnabledTraitLoc, UNTAG_RESOURCE_OPNAME)); + events.add(getMissingOperationEvent(service, trait, UNTAG_RESOURCE_OPNAME)); } - Optional listTagResourceId = awsTagIndex.getListTagsForResourceOperation(service.getId()); - if (listTagResourceId.isPresent()) { + Optional listTagsId = awsTagIndex.getListTagsForResourceOperation(service.getId()); + if (listTagsId.isPresent()) { if (!awsTagIndex.serviceHasValidListTagsForResourceOperation(service.getId())) { - events.add(getMessageUnqualifedOperation(service, tagEnabledTraitLoc, - listTagResourceId.get(), LISTTAGS_OPNAME)); + events.add(getInvalidOperationEvent(service, trait, listTagsId.get(), LIST_TAGS_OPNAME)); } } else { - events.add(getMessageMissingOperation(service, tagEnabledTraitLoc, LISTTAGS_OPNAME)); + events.add(getMissingOperationEvent(service, trait, LIST_TAGS_OPNAME)); } return events; } - private ValidationEvent getMessageMissingOperation( - ServiceShape service, - SourceLocation location, - String opName - ) { + private ValidationEvent getMissingOperationEvent(ServiceShape service, FromSourceLocation location, String opName) { return warning(service, location, "Service marked `aws.api#TagEnabled` is missing an operation named " - + "'" + opName + ".'"); + + "'" + opName + ".'"); } - private ValidationEvent getMessageUnqualifedOperation( + private ValidationEvent getInvalidOperationEvent( ServiceShape service, - SourceLocation location, + FromSourceLocation location, ShapeId opId, String opName ) { diff --git a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TagEnabledServiceValidator.java b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TagEnabledServiceValidator.java index 71b54059c78..5984d14e4b0 100644 --- a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TagEnabledServiceValidator.java +++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TagEnabledServiceValidator.java @@ -34,13 +34,12 @@ public List validate(Model model) { TopDownIndex topDownIndex = TopDownIndex.of(model); AwsTagIndex tagIndex = AwsTagIndex.of(model); for (ServiceShape service : model.getServiceShapesWithTrait(TagEnabledTrait.class)) { - events.addAll(validateService(model, service, tagIndex, topDownIndex)); + events.addAll(validateService(service, tagIndex, topDownIndex)); } return events; } private List validateService( - Model model, ServiceShape service, AwsTagIndex tagIndex, TopDownIndex topDownIndex diff --git a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TagResourcePropertyNameValidator.java b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TagResourcePropertyNameValidator.java index c6414c77bf2..531e72d5fd2 100644 --- a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TagResourcePropertyNameValidator.java +++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TagResourcePropertyNameValidator.java @@ -32,9 +32,7 @@ public List validate(Model model) { for (ResourceShape resource : model.getResourceShapesWithTrait(TaggableTrait.class)) { TaggableTrait trait = resource.expectTrait(TaggableTrait.class); - if (trait.getProperty() - .filter(property -> !TaggingShapeUtils.isTagDesiredName(property)) - .isPresent()) { + if (trait.getProperty().isPresent() && !TaggingShapeUtils.isTagDesiredName(trait.getProperty().get())) { events.add(warning(resource, String.format("Suggested tag property name is '%s'.", TaggingShapeUtils.getDesiredTagsPropertyName()))); } diff --git a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TagResourcePropertyTypeValidator.java b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TagResourcePropertyTypeValidator.java index 0c1fea782c4..a1d5351cd40 100644 --- a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TagResourcePropertyTypeValidator.java +++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TagResourcePropertyTypeValidator.java @@ -36,14 +36,11 @@ public List validate(Model model) { for (ResourceShape resource : model.getResourceShapesWithTrait(TaggableTrait.class)) { TaggableTrait trait = resource.expectTrait(TaggableTrait.class); Map properties = resource.getProperties(); - if (trait.getProperty().isPresent()) { - ShapeId propertyShapeId = properties.get(trait.getProperty().get()); - if (propertyShapeId != null) { - Shape propertyShape = model.expectShape(propertyShapeId); - if (!TaggingShapeUtils.verifyTagsShape(model, propertyShape)) { - events.add(error(resource, "Tag property must be a list shape targeting a member" - + " containing a pair of strings, or a Map shape targeting a string member.")); - } + if (trait.getProperty().isPresent() && properties.containsKey(trait.getProperty().get())) { + Shape propertyShape = model.expectShape(properties.get(trait.getProperty().get())); + if (!TaggingShapeUtils.verifyTagsShape(model, propertyShape)) { + events.add(error(resource, "Tag property must be a list shape targeting a member" + + " containing a pair of strings, or a Map shape targeting a string member.")); } } } diff --git a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TaggableResourceValidator.java b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TaggableResourceValidator.java index 0508d152835..417bc1627e6 100644 --- a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TaggableResourceValidator.java +++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TaggableResourceValidator.java @@ -24,7 +24,6 @@ import java.util.function.Predicate; import software.amazon.smithy.aws.traits.ArnTrait; import software.amazon.smithy.model.Model; -import software.amazon.smithy.model.knowledge.PropertyBindingIndex; import software.amazon.smithy.model.knowledge.TopDownIndex; import software.amazon.smithy.model.shapes.MemberShape; import software.amazon.smithy.model.shapes.OperationShape; @@ -44,11 +43,10 @@ public List validate(Model model) { List events = new LinkedList<>(); TopDownIndex topDownIndex = TopDownIndex.of(model); AwsTagIndex tagIndex = AwsTagIndex.of(model); - PropertyBindingIndex propertyBindingIndex = PropertyBindingIndex.of(model); for (ServiceShape service : model.getServiceShapesWithTrait(TagEnabledTrait.class)) { for (ResourceShape resource : topDownIndex.getContainedResources(service)) { if (resource.hasTrait(TaggableTrait.class)) { - events.addAll(validateResource(model, resource, service, tagIndex, propertyBindingIndex)); + events.addAll(validateResource(model, resource, service, tagIndex)); } else if (resource.hasTrait(ArnTrait.class) && tagIndex.serviceHasTagApis(service.getId())) { // 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. @@ -63,14 +61,15 @@ private List validateResource( Model model, ResourceShape resource, ServiceShape service, - AwsTagIndex awsTagIndex, - PropertyBindingIndex propertyBindingIndex + AwsTagIndex awsTagIndex ) { List events = new LinkedList<>(); // Generate danger if resource has tag property in update API. if (awsTagIndex.isResourceTagOnUpdate(resource.getId())) { - Shape updateOperation = model.expectShape(resource.getUpdate().get()); - events.add(danger(updateOperation, "Update resource lifecycle operation should not support updating tags" + Shape operation = resource.getUpdate().isPresent() + ? model.expectShape(resource.getUpdate().get()) + : model.expectShape(resource.getPut().get()); + events.add(danger(operation, "Update and put resource lifecycle operations should not support updating tags" + " because it is a privileged operation that modifies access.")); } // A valid taggable resource must support one of the following: @@ -81,8 +80,7 @@ private List validateResource( // through the tag property, and must be resource instance operations //Caution: avoid short circuiting behavior. boolean isServiceWideTaggable = awsTagIndex.serviceHasTagApis(service.getId()); - boolean isInstanceOpTaggable = isTaggableViaInstanceOperations(events, model, resource, service, - propertyBindingIndex); + boolean isInstanceOpTaggable = isTaggableViaInstanceOperations(model, resource); if (isServiceWideTaggable && !isInstanceOpTaggable && !resource.hasTrait(ArnTrait.class)) { events.add(error(resource, "Resource is taggable only via service-wide tag operations." @@ -98,16 +96,10 @@ private List validateResource( } private Optional resolveTagOperation(ShapeId tagApiId, Model model) { - return model.getShape(tagApiId).flatMap(shape -> shape.asOperationShape()); + return model.getShape(tagApiId).flatMap(Shape::asOperationShape); } - private boolean isTaggableViaInstanceOperations( - List events, - Model model, - ResourceShape resource, - ServiceShape service, - PropertyBindingIndex propertyBindingIndex - ) { + private boolean isTaggableViaInstanceOperations(Model model, ResourceShape resource) { TaggableTrait taggableTrait = resource.expectTrait(TaggableTrait.class); if (taggableTrait.getApiConfig().isPresent()) { TaggableApiConfig apiConfig = taggableTrait.getApiConfig().get(); @@ -117,20 +109,19 @@ private boolean isTaggableViaInstanceOperations( Optional tagApi = resolveTagOperation(apiConfig.getTagApi(), model); if (tagApi.isPresent()) { - tagApiVerified = TaggingShapeUtils.isTagPropertyInInput(Optional.of( - tagApi.get().getId()), model, resource, propertyBindingIndex) - && verifyTagApi(tagApi.get(), model, service, resource); + tagApiVerified = TaggingShapeUtils.isTagPropertyInInput( + Optional.of(tagApi.get().getId()), model, resource) + && verifyTagApi(tagApi.get(), model); } Optional untagApi = resolveTagOperation(apiConfig.getUntagApi(), model); if (untagApi.isPresent()) { - untagApiVerified = verifyUntagApi(untagApi.get(), model, service, resource); + untagApiVerified = verifyUntagApi(untagApi.get(), model); } - Optional listTagsApi = resolveTagOperation(apiConfig.getListTagsApi(), model); if (listTagsApi.isPresent()) { - listTagsApiVerified = verifyListTagsApi(listTagsApi.get(), model, service, resource); + listTagsApiVerified = verifyListTagsApi(listTagsApi.get(), model); } return tagApiVerified && untagApiVerified && listTagsApiVerified; @@ -138,41 +129,26 @@ private boolean isTaggableViaInstanceOperations( return false; } - private boolean verifyListTagsApi( - OperationShape listTagsApi, - Model model, - ServiceShape service, - ResourceShape resource - ) { + private boolean verifyListTagsApi(OperationShape listTagsApi, Model model) { // Verify Tags map or list member but on the output. return exactlyOne(collectMemberTargetShapes(listTagsApi.getOutputShape(), model), memberEntry -> TaggingShapeUtils.isTagDesiredName(memberEntry.getKey().getMemberName()) - && TaggingShapeUtils.verifyTagsShape(model, memberEntry.getValue())); + && TaggingShapeUtils.verifyTagsShape(model, memberEntry.getValue())); } - private boolean verifyUntagApi( - OperationShape untagApi, - Model model, - ServiceShape service, - ResourceShape resource - ) { - // Tag API has a tags property on its input AND has exactly one member targetting a tag shape with an + private boolean verifyUntagApi(OperationShape untagApi, Model model) { + // Tag API has a tags property on its input AND has exactly one member targeting a tag shape with an // appropriate name. return exactlyOne(collectMemberTargetShapes(untagApi.getInputShape(), model), memberEntry -> TaggingShapeUtils.isTagKeysDesiredName(memberEntry.getKey().getMemberName()) - && TaggingShapeUtils.verifyTagKeysShape(model, memberEntry.getValue())); + && TaggingShapeUtils.verifyTagKeysShape(model, memberEntry.getValue())); } - private boolean verifyTagApi( - OperationShape tagApi, - Model model, - ServiceShape service, - ResourceShape resource - ) { - // Tag API has exactly one member targetting a tag list or map shape with an appropriate name. + private boolean verifyTagApi(OperationShape tagApi, Model model) { + // Tag API has exactly one member targeting a tag list or map shape with an appropriate name. return exactlyOne(collectMemberTargetShapes(tagApi.getInputShape(), model), memberEntry -> TaggingShapeUtils.isTagDesiredName(memberEntry.getKey().getMemberName()) - && TaggingShapeUtils.verifyTagsShape(model, memberEntry.getValue())); + && TaggingShapeUtils.verifyTagsShape(model, memberEntry.getValue())); } private boolean exactlyOne( diff --git a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TaggingShapeUtils.java b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TaggingShapeUtils.java index 27672ce7f52..5c2d3857d3e 100644 --- a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TaggingShapeUtils.java +++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TaggingShapeUtils.java @@ -26,7 +26,6 @@ import software.amazon.smithy.model.shapes.MemberShape; import software.amazon.smithy.model.shapes.OperationShape; import software.amazon.smithy.model.shapes.ResourceShape; -import software.amazon.smithy.model.shapes.ServiceShape; import software.amazon.smithy.model.shapes.Shape; import software.amazon.smithy.model.shapes.ShapeId; import software.amazon.smithy.model.shapes.StructureShape; @@ -80,7 +79,7 @@ static boolean isTagKeysDesiredName(String memberName) { static boolean hasResourceArnInput(Map inputMembers, Model model) { for (Map.Entry memberEntry : inputMembers.entrySet()) { - if (TaggingShapeUtils.isArnMemberDesiredName(memberEntry.getKey()) + if (isArnMemberDesiredName(memberEntry.getKey()) && model.expectShape(memberEntry.getValue().getTarget()).isStringShape()) { return true; } @@ -107,7 +106,7 @@ static boolean verifyTagListShape(Model model, Shape tagShape) { Shape listTargetShape = model.expectShape(listShape.getMember().getTarget()); if (listTargetShape.isStructureShape()) { StructureShape memberStructureShape = listTargetShape.asStructureShape().get(); - //Verify member count is two, and both point to string types. + // Verify member count is two, and both point to string types. if (memberStructureShape.members().size() == 2) { boolean allStrings = true; for (MemberShape member : memberStructureShape.members()) { @@ -136,43 +135,40 @@ static boolean verifyTagKeysShape(Model model, Shape tagShape) { } static boolean verifyTagResourceOperation(Model model, - ServiceShape service, OperationShape tagResourceOperation, OperationIndex operationIndex ) { Map inputMembers = operationIndex.getInputMembers(tagResourceOperation); int taglistMemberCount = 0; for (Map.Entry memberEntry : inputMembers.entrySet()) { - if (TaggingShapeUtils.isTagDesiredName(memberEntry.getKey()) - && TaggingShapeUtils.verifyTagsShape(model, - model.expectShape(memberEntry.getValue().getTarget()))) { + if (isTagDesiredName(memberEntry.getKey()) + && verifyTagsShape(model, model.expectShape(memberEntry.getValue().getTarget())) + ) { ++taglistMemberCount; } } - return taglistMemberCount == 1 && TaggingShapeUtils.hasResourceArnInput(inputMembers, model); + return taglistMemberCount == 1 && hasResourceArnInput(inputMembers, model); } static boolean verifyUntagResourceOperation( Model model, - ServiceShape service, OperationShape untagResourceOperation, OperationIndex operationIndex ) { Map inputMembers = operationIndex.getInputMembers(untagResourceOperation); int untagKeyMemberCount = 0; for (Map.Entry memberEntry : inputMembers.entrySet()) { - if (TaggingShapeUtils.isTagKeysDesiredName(memberEntry.getKey()) - && TaggingShapeUtils.verifyTagKeysShape(model, - model.expectShape(memberEntry.getValue().getTarget()))) { + if (isTagKeysDesiredName(memberEntry.getKey()) + && verifyTagKeysShape(model, model.expectShape(memberEntry.getValue().getTarget())) + ) { ++untagKeyMemberCount; } } - return untagKeyMemberCount == 1 && TaggingShapeUtils.hasResourceArnInput(inputMembers, model); + return untagKeyMemberCount == 1 && hasResourceArnInput(inputMembers, model); } static boolean verifyListTagsOperation( Model model, - ServiceShape service, OperationShape listTagsResourceOperation, OperationIndex operationIndex ) { @@ -180,21 +176,21 @@ static boolean verifyListTagsOperation( Map outputMembers = operationIndex.getOutputMembers(listTagsResourceOperation); int taglistMemberCount = 0; for (Map.Entry memberEntry : outputMembers.entrySet()) { - if (TaggingShapeUtils.isTagDesiredName(memberEntry.getKey()) - && TaggingShapeUtils.verifyTagsShape(model, - model.expectShape(memberEntry.getValue().getTarget()))) { + if (isTagDesiredName(memberEntry.getKey()) + && verifyTagsShape(model, model.expectShape(memberEntry.getValue().getTarget())) + ) { ++taglistMemberCount; } } - return taglistMemberCount == 1 && TaggingShapeUtils.hasResourceArnInput(inputMembers, model); + return taglistMemberCount == 1 && hasResourceArnInput(inputMembers, model); } static boolean isTagPropertyInInput( Optional operationId, Model model, - ResourceShape resource, - PropertyBindingIndex propertyBindingIndex + ResourceShape resource ) { + PropertyBindingIndex propertyBindingIndex = PropertyBindingIndex.of(model); Optional property = resource.expectTrait(TaggableTrait.class).getProperty(); if (property.isPresent()) { if (operationId.isPresent()) { @@ -206,32 +202,14 @@ static boolean isTagPropertyInInput( return false; } - static boolean isTagPropertyInOutput( - Optional operationId, - Model model, - ResourceShape resource, - PropertyBindingIndex propertyBindingIndex - ) { - Optional property = resource.expectTrait(TaggableTrait.class).getProperty(); - if (property.isPresent()) { - if (operationId.isPresent()) { - OperationShape operation = model.expectShape(operationId.get()).asOperationShape().get(); - Shape outputShape = model.expectShape(operation.getOutputShape()); - return isTagPropertyInShape(property.get(), outputShape, propertyBindingIndex); - } - } - return false; - } - static boolean isTagPropertyInShape( String tagPropertyName, Shape shape, PropertyBindingIndex propertyBindingIndex ) { for (MemberShape member : shape.members()) { - Optional isMatch = propertyBindingIndex.getPropertyName(member.getId()) - .map(name -> name.equals(tagPropertyName)); - if (isMatch.isPresent() && isMatch.get()) { + Optional propertyName = propertyBindingIndex.getPropertyName(member.getId()); + if (propertyName.isPresent() && propertyName.get().equals(tagPropertyName)) { return true; } } diff --git a/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/tagging/AwsTagIndexTest.java b/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/tagging/AwsTagIndexTest.java new file mode 100644 index 00000000000..01f6f512186 --- /dev/null +++ b/smithy-aws-traits/src/test/java/software/amazon/smithy/aws/traits/tagging/AwsTagIndexTest.java @@ -0,0 +1,112 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.aws.traits.tagging; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Optional; +import java.util.stream.Stream; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.shapes.MemberShape; +import software.amazon.smithy.model.shapes.ShapeId; + +public final class AwsTagIndexTest { + private static final String NAMESPACE = "example.weather"; + private static final ShapeId WEATHER_SERVICE_ID = ShapeId.fromParts(NAMESPACE, "Weather"); + private static final ShapeId UNTAGGED_SERVICE_ID = ShapeId.fromParts(NAMESPACE, "UntaggedService"); + private static final ShapeId CITY_RESOURCE_ID = ShapeId.fromParts(NAMESPACE, "City"); + + private static Model model; + private static AwsTagIndex tagIndex; + + @BeforeAll + public static void loadModel() { + model = Model.assembler() + .addImport(AwsTagIndex.class.getResource("aws-tag-index-test-model.smithy")) + .discoverModels() + .assemble() + .unwrap(); + tagIndex = AwsTagIndex.of(model); + } + + @Test + public void detectsCompliantTaggingService() { + assertTrue(tagIndex.serviceHasTagApis(WEATHER_SERVICE_ID)); + assertTrue(tagIndex.serviceHasValidTagResourceOperation(WEATHER_SERVICE_ID)); + assertTrue(tagIndex.serviceHasValidUntagResourceOperation(WEATHER_SERVICE_ID)); + assertTrue(tagIndex.serviceHasValidListTagsForResourceOperation(WEATHER_SERVICE_ID)); + } + + @Test + public void detectsNoncompliantTaggingService() { + assertFalse(tagIndex.serviceHasTagApis(UNTAGGED_SERVICE_ID)); + assertFalse(tagIndex.serviceHasValidTagResourceOperation(UNTAGGED_SERVICE_ID)); + assertFalse(tagIndex.serviceHasValidUntagResourceOperation(UNTAGGED_SERVICE_ID)); + assertFalse(tagIndex.serviceHasValidListTagsForResourceOperation(UNTAGGED_SERVICE_ID)); + } + + @Test + public void detectsDefaultTagOperations() { + Optional tagOptional = tagIndex.getTagResourceOperation(WEATHER_SERVICE_ID); + assertTrue(tagOptional.isPresent()); + assertEquals(tagOptional.get(), ShapeId.fromParts(NAMESPACE, "TagResource")); + + Optional untagOptional = tagIndex.getUntagResourceOperation(WEATHER_SERVICE_ID); + assertTrue(untagOptional.isPresent()); + assertEquals(untagOptional.get(), ShapeId.fromParts(NAMESPACE, "UntagResource")); + + Optional listTagsOptional = tagIndex.getListTagsForResourceOperation(WEATHER_SERVICE_ID); + assertTrue(listTagsOptional.isPresent()); + assertEquals(listTagsOptional.get(), ShapeId.fromParts(NAMESPACE, "ListTagsForResource")); + } + + @Test + public void detectsResourceCustomizedTagOperations() { + Optional tagOptional = tagIndex.getTagResourceOperation(CITY_RESOURCE_ID); + assertTrue(tagOptional.isPresent()); + assertEquals(tagOptional.get(), ShapeId.fromParts(NAMESPACE, "TagCity")); + + Optional untagOptional = tagIndex.getUntagResourceOperation(CITY_RESOURCE_ID); + assertTrue(untagOptional.isPresent()); + assertEquals(untagOptional.get(), ShapeId.fromParts(NAMESPACE, "UntagCity")); + + Optional listTagsOptional = tagIndex.getListTagsForResourceOperation(CITY_RESOURCE_ID); + assertTrue(listTagsOptional.isPresent()); + assertEquals(listTagsOptional.get(), ShapeId.fromParts(NAMESPACE, "ListTagsForCity")); + } + + @ParameterizedTest + @MethodSource("resourceTagMutabilities") + public void detectsResourceTagMutation(ShapeId shapeId, boolean isTagOnCreate, boolean isTagOnUpdate) { + assertEquals(tagIndex.isResourceTagOnCreate(shapeId), isTagOnCreate); + assertEquals(tagIndex.isResourceTagOnUpdate(shapeId), isTagOnUpdate); + } + + public static Stream resourceTagMutabilities() { + return Stream.of( + Arguments.of(CITY_RESOURCE_ID, false, false), + Arguments.of(ShapeId.fromParts(NAMESPACE, "Town"), true, true), + Arguments.of(ShapeId.fromParts(NAMESPACE, "Farm"), true, true), + Arguments.of(ShapeId.fromParts(NAMESPACE, "Barn"), true, false), + Arguments.of(ShapeId.fromParts(NAMESPACE, "Silo"), true, true)); + } + + @Test + public void resolvesTagMember() { + assertFalse(tagIndex.getTagsMember(ShapeId.fromParts(NAMESPACE, "GetCity")).isPresent()); + + Optional tagMember = tagIndex.getTagsMember(ShapeId.fromParts(NAMESPACE, "CreateTown")); + assertTrue(tagMember.isPresent()); + assertEquals(tagMember.get().toShapeId(), ShapeId.fromParts(NAMESPACE, "CreateTownInput", "tags")); + } +} diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/tagging-warnings.errors b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/tagging-warnings.errors index 5e766be81f3..eb96150771a 100644 --- a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/tagging-warnings.errors +++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/tagging-warnings.errors @@ -2,4 +2,4 @@ [WARNING] example.weather#Weather: Service marked `aws.api#TagEnabled` is missing an operation named 'TagResource.' | ServiceTagging [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 resource lifecycle operation should not support updating tags because it is a privileged operation that modifies access. | TaggableResource +[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 diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/tagging/aws-tag-index-test-model.smithy b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/tagging/aws-tag-index-test-model.smithy new file mode 100644 index 00000000000..990a788f24e --- /dev/null +++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/tagging/aws-tag-index-test-model.smithy @@ -0,0 +1,321 @@ +$version: "2.0" + +metadata suppressions = [ + { + id: "UnstableTrait", + namespace: "example.weather" + } +] + +namespace example.weather + +use aws.api#taggable +use aws.api#arn +use aws.api#tagEnabled + +@tagEnabled +service Weather { + version: "2006-03-01", + resources: [ + City + Town + Farm + Barn + ] + operations: [GetCurrentTime, TagResource, UntagResource, ListTagsForResource] +} + +service UntaggedService {} + +structure Tag { + key: String + value: String +} + +list TagList { + member: Tag +} + +list TagKeys { + member: String +} + +operation TagResource { + input := { + @required + arn: String + @length(max: 128) + tags: TagList + } + output := { } +} + +operation UntagResource { + input := { + @required + arn: String + @required + tagKeys: TagKeys + } + output := { } +} + +operation ListTagsForResource { + input := { + @required + arn: String + } + output := { + @length(max: 128) + tags: TagList + } +} + +operation TagCity { + input := { + @required + cityId: CityId + @length(max: 128) + tags: TagList + } + output := { } +} + +operation UntagCity { + input := { + @required + cityId: CityId + @required + @notProperty + tagKeys: TagKeys + } + output := { } +} + +operation ListTagsForCity { + input := { + @required + cityId: CityId + } + output := { + @length(max: 128) + tags: TagList + } +} + +@taggable(property: "tags", apiConfig: {tagApi: TagCity, untagApi: UntagCity, listTagsApi: ListTagsForCity}) +resource City { + identifiers: { cityId: CityId }, + properties: { + name: String + coordinates: CityCoordinates + tags: TagList + } + create: CreateCity + read: GetCity, + operations: [TagCity, UntagCity, ListTagsForCity], +} + +operation CreateCity { + input := { + name: String + coordinates: CityCoordinates + } + output := { + @required + cityId: CityId + } +} + +@pattern("^[A-Za-z0-9 ]+$") +string CityId + +@readonly +operation GetCity { + input: GetCityInput, + output: GetCityOutput, + errors: [NoSuchResource] +} + +@input +structure GetCityInput { + // "cityId" provides the identifier for the resource and + // has to be marked as required. + @required + cityId: CityId +} + +@output +structure GetCityOutput { + // "required" is used on output to indicate if the service + // will always provide a value for the member. + @required + name: String, + + @required + coordinates: CityCoordinates, +} + +// This structure is nested within GetCityOutput. +structure CityCoordinates { + @required + latitude: Float, + + @required + longitude: Float, +} + +// "error" is a trait that is used to specialize +// a structure as an error. +@error("client") +structure NoSuchResource { + @required + resourceType: String +} + +@readonly +operation GetCurrentTime { + input: GetCurrentTimeInput, + output: GetCurrentTimeOutput +} + +@input +structure GetCurrentTimeInput {} + +@output +structure GetCurrentTimeOutput { + @required + time: Timestamp +} + +// Create and Update +@arn(template: "town/{townId}") +@taggable(property: "tags") +resource Town { + identifiers: { townId: String } + properties: { + name: String + tags: TagList + } + create: CreateTown + update: UpdateTown +} + +operation CreateTown { + input := for Town { + $name + $tags + } + output := for Town { + @required + $townId + } +} + +@suppress(["TaggableResource"]) +operation UpdateTown { + input := for Town { + @required + $townId + + $name + $tags + } + output := for Town { + @required + $townId + } +} + +// Put +@taggable(property: "tags") +@arn(template: "farm/{farmId}") +resource Farm { + identifiers: { farmId: String } + properties: { + name: String + tags: TagList + } + put: PutFarm + resources: [Silo] +} + +@idempotent +@suppress(["TaggableResource"]) +operation PutFarm { + input := for Farm { + @required + $farmId + + $name + $tags + } + output := for Farm { + @required + $farmId + } +} + +// Put with noReplace +@arn(template: "barn/{barnId}") +@noReplace +@taggable(property: "tags") +resource Barn { + identifiers: { barnId: String } + properties: { + name: String + tags: TagList + } + put: PutBarn +} + +@idempotent +operation PutBarn { + input := for Barn { + @required + $barnId + + $name + $tags + } + output := for Barn { + @required + $barnId + } +} + +@taggable(property: "tags") +@arn(template: "silo/{siloId}") +resource Silo { + identifiers: { + farmId: String + siloId: String + } + properties: { + name: String + tags: TagList + } + put: PutSilo +} + +@idempotent +@suppress(["TaggableResource"]) +operation PutSilo { + input := for Silo { + @required + $farmId + + @required + $siloId + + $name + $tags + } + output := for Silo { + @required + $farmId + + @required + $siloId + } +}