Repository: smithy-lang/smithy
Pull request #2382 (2024-08-26 19:29:21)

Update Endpoint AuthScheme docs to reflect current auth resolution process

#### Background
The existing docs are out of date and do not reflect the current process SDK's use to resolve authentication schemes and this has caused service team confusion.

#### Testing
* `make html`

---
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/docs/source-2.0/additional-specs/rules-engine/specification.rst b/docs/source-2.0/additional-specs/rules-engine/specification.rst
index 8907e4d0a..f1861f976 100644
--- a/docs/source-2.0/additional-specs/rules-engine/specification.rst
+++ b/docs/source-2.0/additional-specs/rules-engine/specification.rst
@@ -297,29 +297,40 @@ booleans.
 
 .. note::
     To prevent ambiguity, the endpoint properties map MUST NOT contain
-    reference or function objects. Properties MAY contain :ref:`template string <rules-engine-endpoint-rule-set-template-string>`
+    reference or function objects. Properties MAY contain :ref:`template
+    string <rules-engine-endpoint-rule-set-template-string>`
 
 .. _rules-engine-endpoint-rule-set-endpoint-authschemes:
 
 Endpoint ``authSchemes`` list property
 --------------------------------------
 
-The ``authSchemes`` property of an endpoint is used to specify the priority
-ordered list of authentication schemes and their configuration supported by the
-endpoint. The property is a list of configuration objects that MUST contain at
-least a ``name`` property and MAY contain additional properties. Each
-configuration object MUST have a unique value for its ``name`` property within
-the list of configuration objects within a given ``authSchemes`` property.
+The ``authSchemes`` property of an endpoint is used to specify a list of
+authentication schemes and their configuration. These are used to modify
+properties of the resolved and modeled authentication scheme. Clients SHOULD
+resolve the authentication scheme following the service's :ref:`auth trait
+<auth-trait>` and SHOULD NOT use the endpoint's ``authSchemes`` property to
+determine which authentication scheme to use. Clients SHOULD use the
+endpoint's ``authSchemes`` property to modify signing properties of the
+resolved authentication scheme.
+
+The property is a list of configuration objects that MUST contain at least a
+``name`` property and MAY contain additional properties. Each configuration
+object MUST have a unique value for its ``name`` property within the list of
+configuration objects within a given ``authSchemes`` property.
 
 If an ``authSchemes`` property is present on an `Endpoint object`_, clients
-MUST resolve an authentication scheme to use via the following process:
+SHOULD modify the signing properties of the resolved and modeled authentication
+scheme via the following process:
 
 #. Iterate through configuration objects in the ``authSchemes`` property.
-#. If the ``name`` property in a configuration object contains a supported
-   authentication scheme, resolve this scheme.
-#. If the ``name`` is unknown or unsupported, ignore it and continue iterating.
-#. If the list has been fully iterated and no scheme has been resolved, clients
-   MUST return an error.
+#. If the ``name`` property in a configuration object matches the resolved
+   authentication scheme, update the resolved authentication and signing
+   properties from the matching ``authSchemes`` properties.
+#. If the ``name`` does not match, ignore it and continue iterating.
+#. If the list has been fully iterated and no scheme has matched, do not
+   modify the resolved authentication scheme's properties
+   and do not raise an error.
 
 .. _rules-engine-standard-library-adding-authscheme-validators:
 
diff --git a/smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/HandlerPermissionMapper.java b/smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/HandlerPermissionMapper.java
index a32bbc516..9a449c98a 100644
--- a/smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/HandlerPermissionMapper.java
+++ b/smithy-aws-cloudformation/src/main/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/HandlerPermissionMapper.java
@@ -67,10 +67,9 @@ public final class HandlerPermissionMapper implements CfnMapper {
                 .orElse(SetUtils.of());
         createPermissions.addAll(putPermissions);
         // Put operations without the noReplace trait are used for updates.
-        resource.getPut()
-                .map(model::expectShape)
-                .filter(shape -> !shape.hasTrait(NoReplaceTrait.class))
-                .ifPresent(shape -> updatePermissions.addAll(putPermissions));
+        if (!resource.hasTrait(NoReplaceTrait.class)) {
+            updatePermissions.addAll(putPermissions);
+        }
 
         // Set the create and update handlers, if they have permissions, now that they're complete.
         if (!createPermissions.isEmpty()) {
diff --git a/smithy-model/src/jmh/java/software/amazon/smithy/model/jmh/KnowledgeIndicies.java b/smithy-model/src/jmh/java/software/amazon/smithy/model/jmh/KnowledgeIndicies.java
new file mode 100644
index 000000000..0b5b397d1
--- /dev/null
+++ b/smithy-model/src/jmh/java/software/amazon/smithy/model/jmh/KnowledgeIndicies.java
@@ -0,0 +1,46 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package software.amazon.smithy.model.jmh;
+
+import java.util.concurrent.TimeUnit;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Warmup;
+import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.knowledge.BottomUpIndex;
+
+@Warmup(iterations = 3)
+@Measurement(iterations = 3, timeUnit = TimeUnit.MICROSECONDS)
+@BenchmarkMode(Mode.AverageTime)
+@Fork(1)
+public class KnowledgeIndicies {
+    @State(Scope.Thread)
+    public static class IndiciesState {
+        public Model model;
+
+        @Setup
+        public void prepare() {
+            model = Model.assembler()
+                    .addImport(KnowledgeIndicies.class.getResource("test-model.smithy"))
+                    .disableValidation()
+                    .assemble()
+                    .getResult()
+                    .get();
+        }
+    }
+
+    @Benchmark
+    public BottomUpIndex createsBottomUpIndex(IndiciesState state) {
+        // Use the ctor so the index doesn't get cached in warmup runs and re-used
+        return new BottomUpIndex(state.model);
+    }
+}
diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/BottomUpIndex.java b/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/BottomUpIndex.java
index 5c157c281..784788c45 100644
--- a/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/BottomUpIndex.java
+++ b/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/BottomUpIndex.java
@@ -15,47 +15,62 @@
 
 package software.amazon.smithy.model.knowledge;
 
-import java.util.ArrayList;
+import java.util.ArrayDeque;
 import java.util.Collections;
+import java.util.Deque;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.neighbor.NeighborProvider;
 import software.amazon.smithy.model.neighbor.Relationship;
-import software.amazon.smithy.model.selector.PathFinder;
-import software.amazon.smithy.model.selector.Selector;
 import software.amazon.smithy.model.shapes.EntityShape;
 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.ToShapeId;
+import software.amazon.smithy.utils.ListUtils;
 
 /**
  * Computes all of the parent shapes of resources and operations from the bottom-up.
  */
 public final class BottomUpIndex implements KnowledgeIndex {
-    private static final Selector SELECTOR = Selector.parse(":is(resource, operation)");
     private final Map<ShapeId, Map<ShapeId, List<EntityShape>>> parentBindings = new HashMap<>();
 
     public BottomUpIndex(Model model) {
-        PathFinder pathFinder = PathFinder.create(model);
-
+        NeighborProvider provider = NeighborProviderIndex.of(model).getProvider();
         for (ServiceShape service : model.getServiceShapes()) {
-            Map<ShapeId, List<EntityShape>> serviceBindings = new HashMap<>();
-            parentBindings.put(service.getId(), serviceBindings);
-            for (PathFinder.Path path : pathFinder.search(service, SELECTOR)) {
-                List<EntityShape> shapes = new ArrayList<>();
-                // Get all of the path elements other than the last one in reverse order.
-                for (int i = path.size() - 1; i >= 0; i--) {
-                    Relationship rel = path.get(i);
-                    // This should always be an EntityShape, but just in case new relationships are added...
-                    if (rel.getShape() instanceof EntityShape) {
-                        shapes.add((EntityShape) rel.getShape());
-                    }
-                }
-                // Add the end shape (a resource or operation) to the service bindings.
-                serviceBindings.put(path.getEndShape().getId(), shapes);
+            Map<ShapeId, List<EntityShape>> paths = new HashMap<>();
+            parentBindings.put(service.getId(), paths);
+
+            Deque<EntityShape> path = new ArrayDeque<>();
+            path.push(service);
+            collectPaths(paths, path, service, provider);
+        }
+    }
+
+    private void collectPaths(
+            Map<ShapeId, List<EntityShape>> paths,
+            Deque<EntityShape> path,
+            Shape current,
+            NeighborProvider neighborProvider
+    ) {
+        for (Relationship relationship : neighborProvider.getNeighbors(current)) {
+            Shape neighbor = relationship.expectNeighborShape();
+            if (!neighbor.isOperationShape() && !neighbor.isResourceShape()) {
+                continue;
+            }
+
+            // Note: The path does not include the neighbor shape
+            paths.put(neighbor.getId(), ListUtils.copyOf(path));
+
+            // Recurse through neighbors of an entity (resource) shape
+            if (neighbor instanceof EntityShape) {
+                path.push((EntityShape) neighbor);
+                collectPaths(paths, path, neighbor, neighborProvider);
+                path.pop();
             }
         }
     }


Test patch:
diff --git a/smithy-aws-cloudformation/src/test/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/DocumentationMapperTest.java b/smithy-aws-cloudformation/src/test/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/DocumentationMapperTest.java
index 6be933da9..dfff9d670 100644
--- a/smithy-aws-cloudformation/src/test/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/DocumentationMapperTest.java
+++ b/smithy-aws-cloudformation/src/test/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/DocumentationMapperTest.java
@@ -16,6 +16,7 @@
 package software.amazon.smithy.aws.cloudformation.schema.fromsmithy.mappers;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.List;
@@ -47,12 +48,8 @@ public class DocumentationMapperTest {
         config.setOrganizationName("Smithy");
         config.setService(ShapeId.from("smithy.example#TestService"));
 
-        List<ResourceSchema> schemas = CfnConverter.create()
-                .config(config)
-                .convert(model);
-
-        assertEquals(1, schemas.size());
-        ResourceSchema schema = schemas.get(0);
+        ResourceSchema schema = getBarSchema(config);
+        assertNotNull(schema);
         assertTrue(schema.getDocumentationUrl().isPresent());
         assertEquals("https://docs.example.com", schema.getDocumentationUrl().get());
         assertTrue(schema.getSourceUrl().isPresent());
@@ -67,15 +64,20 @@ public class DocumentationMapperTest {
         config.setExternalDocs(ListUtils.of("main"));
         config.setSourceDocs(ListUtils.of("code"));
 
-        List<ResourceSchema> schemas = CfnConverter.create()
-                .config(config)
-                .convert(model);
-
-        assertEquals(1, schemas.size());
-        ResourceSchema schema = schemas.get(0);
+        ResourceSchema schema = getBarSchema(config);
+        assertNotNull(schema);
         assertTrue(schema.getDocumentationUrl().isPresent());
         assertEquals("https://docs2.example.com", schema.getDocumentationUrl().get());
         assertTrue(schema.getSourceUrl().isPresent());
         assertEquals("https://source2.example.com", schema.getSourceUrl().get());
     }
+
+    private ResourceSchema getBarSchema(CfnConfig config) {
+        for (ResourceSchema schema : CfnConverter.create().config(config).convert(model)) {
+            if (schema.getTypeName().endsWith("FooResource")) {
+                return schema;
+            }
+        }
+        return null;
+    }
 }
diff --git a/smithy-aws-cloudformation/src/test/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/HandlerPermissionMapperTest.java b/smithy-aws-cloudformation/src/test/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/HandlerPermissionMapperTest.java
index 1bbe03ca9..3903d0872 100644
--- a/smithy-aws-cloudformation/src/test/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/HandlerPermissionMapperTest.java
+++ b/smithy-aws-cloudformation/src/test/java/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/HandlerPermissionMapperTest.java
@@ -19,9 +19,11 @@ import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.Map;
 import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import software.amazon.smithy.aws.cloudformation.schema.CfnConfig;
 import software.amazon.smithy.aws.cloudformation.schema.fromsmithy.CfnConverter;
@@ -32,24 +34,30 @@ import software.amazon.smithy.model.node.StringNode;
 import software.amazon.smithy.model.shapes.ShapeId;
 
 public final class HandlerPermissionMapperTest {
-    @Test
-    public void addsHandlerPermissionsByDefault() {
-        Model model = Model.assembler()
-                .addImport(HandlerPermissionMapperTest.class.getResource("simple.smithy"))
+
+    private static Model model;
+
+    @BeforeAll
+    public static void loadModel() {
+        model = Model.assembler()
+                .addImport(DocumentationMapperTest.class.getResource("simple.smithy"))
                 .discoverModels()
                 .assemble()
                 .unwrap();
+    }
 
+    private ObjectNode getResourceByName(String resourceName) {
         CfnConfig config = new CfnConfig();
         config.setOrganizationName("Smithy");
         config.setService(ShapeId.from("smithy.example#TestService"));
+        Map<String, ObjectNode> resourceNodes = CfnConverter.create().config(config).convertToNodes(model);
+        return resourceNodes.get(resourceName);
+    }
 
-        ObjectNode resourceNode = CfnConverter.create()
-                .config(config)
-                .convertToNodes(model)
-                .get("Smithy::TestService::FooResource");
-
-        Map<String, Node> handlersDefined = resourceNode.expectObjectMember("handlers").getStringMap();
+    @Test
+    public void addsCRUHandlerPermissionsByDefault() {
+        ObjectNode fooResourceNode = getResourceByName("Smithy::TestService::FooResource");
+        Map<String, Node> handlersDefined = fooResourceNode.expectObjectMember("handlers").getStringMap();
         Assertions.assertEquals(3, handlersDefined.size());
         assertThat(handlersDefined.keySet(), containsInAnyOrder("create", "read", "update"));
 
@@ -63,14 +71,36 @@ public final class HandlerPermissionMapperTest {
                            .expectArrayMember("permissions").getElementsAs(StringNode::getValue),
                 contains("testservice:UpdateFooOperation"));
     }
+
     @Test
-    public void canDisableHandlerPermissionsGeneration() {
-        Model model = Model.assembler()
-                .addImport(HandlerPermissionMapperTest.class.getResource("simple.smithy"))
-                .discoverModels()
-                .assemble()
-                .unwrap();
+    public void addsPutHandlerPermissionsByDefault() {
+        ObjectNode barResourceNode = getResourceByName("Smithy::TestService::BarResource");
+        Map<String, Node> handlersDefined = barResourceNode.expectObjectMember("handlers").getStringMap();
+        Assertions.assertEquals(2, handlersDefined.size());
+        assertThat(handlersDefined.keySet(), containsInAnyOrder("create", "update"));
 
+        assertThat(handlersDefined.get("create").expectObjectNode()
+                        .expectArrayMember("permissions").getElementsAs(StringNode::getValue),
+                contains("testservice:CreateBar"));
+        assertThat(handlersDefined.get("update").expectObjectNode()
+                        .expectArrayMember("permissions").getElementsAs(StringNode::getValue),
+                contains("testservice:CreateBar"));
+    }
+
+    @Test
+    public void addsPutWithNoReplaceHandlerPermissionsByDefault() {
+        ObjectNode bazResourceNode = getResourceByName("Smithy::TestService::BazResource");
+        Map<String, Node> handlersDefined = bazResourceNode.expectObjectMember("handlers").getStringMap();
+        Assertions.assertEquals(1, handlersDefined.size());
+        assertThat(handlersDefined.keySet(), contains("create"));
+
+        assertThat(handlersDefined.get("create").expectObjectNode()
+                        .expectArrayMember("permissions").getElementsAs(StringNode::getValue),
+                contains("testservice:CreateBaz"));
+    }
+
+    @Test
+    public void canDisableHandlerPermissionsGeneration() {
         CfnConfig config = new CfnConfig();
         config.setOrganizationName("Smithy");
         config.setService(ShapeId.from("smithy.example#TestService"));
diff --git a/smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/simple.smithy b/smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/simple.smithy
index edcf04fec..046ec4c93 100644
--- a/smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/simple.smithy
+++ b/smithy-aws-cloudformation/src/test/resources/software/amazon/smithy/aws/cloudformation/schema/fromsmithy/mappers/simple.smithy
@@ -9,6 +9,8 @@ service TestService {
     version: "2020-07-02",
     resources: [
         FooResource,
+        BarResource
+        BazResource
     ],
 }
 
@@ -115,3 +117,50 @@ structure ComplexProperty {
     property: String,
     another: String,
 }
+
+/// The Bar resource is cooler.
+@externalDocumentation(
+    "Documentation Url": "https://docs.example.com",
+    "Source Url": "https://source.example.com",
+    "Main": "https://docs2.example.com",
+    "Code": "https://source2.example.com",
+)
+@cfnResource
+resource BarResource {
+    identifiers: {
+        barId: String
+    }
+    put: CreateBar
+}
+
+@idempotent
+operation CreateBar {
+    input := for BarResource {
+        @required
+        $barId
+    }
+}
+
+/// The Baz resource is irreplaceable.
+@externalDocumentation(
+    "Documentation Url": "https://docs.example.com",
+    "Source Url": "https://source.example.com",
+    "Main": "https://docs2.example.com",
+    "Code": "https://source2.example.com",
+)
+@cfnResource
+@noReplace
+resource BazResource {
+    identifiers: {
+        bazId: String
+    }
+    put: CreateBaz
+}
+
+@idempotent
+operation CreateBaz {
+    input := for BazResource {
+        @required
+        $bazId
+    }
+}