fix: revert "feat: additional options when assuming roles" (#52)

Merging #33 should have bumped the major version of the library, but instead it bumped the minor. > See https://github.com/cdklabs/cloud-assembly-schema/releases/tag/v36.1.0 It shouldn't cause issues but lets revert anyway to avoid surprises while I investigate what happened. Reverts #33
cdklabs · Sep 17, 2024 · 97acb19 · 97acb19
1 parent d3ee265
commit 97acb19
Show file tree

Hide file tree

Showing 8 changed files with 246 additions and 436 deletions.
diff --git a/lib/assets/aws-destination.ts b/lib/assets/aws-destination.ts
@@ -22,15 +22,4 @@ export interface AwsDestination {
    * @default - No ExternalId will be supplied
    */
   readonly assumeRoleExternalId?: string;
-
-  /**
-   * Additional options to pass to STS when assuming the role.
-   *
-   * - `RoleArn` should not be used. Use the dedicated `assumeRoleArn` property instead.
-   * - `ExternalId` should not be used. Use the dedicated `assumeRoleExternalId` instead.
-   *
-   * @see https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/STS.html#assumeRole-property
-   * @default - No additional options.
-   */
-  readonly assumeRoleAdditionalOptions?: { [key: string]: any };
 }
diff --git a/lib/cloud-assembly/artifact-schema.ts b/lib/cloud-assembly/artifact-schema.ts
@@ -16,17 +16,6 @@ export interface BootstrapRole {
    */
   readonly assumeRoleExternalId?: string;
 
-  /**
-   * Additional options to pass to STS when assuming the role.
-   *
-   * - `RoleArn` should not be used. Use the dedicated `arn` property instead.
-   * - `ExternalId` should not be used. Use the dedicated `assumeRoleExternalId` instead.
-   *
-   * @see https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/STS.html#assumeRole-property
-   * @default - No additional options.
-   */
-  readonly assumeRoleAdditionalOptions?: { [key: string]: any };
-
   /**
    * Version of bootstrap stack required to use this role
    *
@@ -92,17 +81,6 @@ export interface AwsCloudFormationStackProperties {
    */
   readonly assumeRoleExternalId?: string;
 
-  /**
-   * Additional options to pass to STS when assuming the role.
-   *
-   * - `RoleArn` should not be used. Use the dedicated `assumeRoleArn` property instead.
-   * - `ExternalId` should not be used. Use the dedicated `assumeRoleExternalId` instead.
-   *
-   * @see https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/STS.html#assumeRole-property
-   * @default - No additional options.
-   */
-  readonly assumeRoleAdditionalOptions?: { [key: string]: any };
-
   /**
    * The role that is passed to CloudFormation to execute the change set
    *

diff --git a/lib/cloud-assembly/context-queries.ts b/lib/cloud-assembly/context-queries.ts
@@ -61,16 +61,16 @@ export enum ContextProvider {
 }
 
 /**
- * Options for context lookup roles.
+ * Query to AMI context provider
  */
-export interface ContextLookupRoleOptions {
+export interface AmiContextQuery {
   /**
-   * Query account
+   * Account to query
    */
   readonly account: string;
 
   /**
-   * Query region
+   * Region to query
    */
   readonly region: string;
 
@@ -81,29 +81,6 @@ export interface ContextLookupRoleOptions {
    */
   readonly lookupRoleArn?: string;
 
-  /**
-   * The ExternalId that needs to be supplied while assuming this role
-   *
-   * @default - No ExternalId will be supplied
-   */
-  readonly lookupRoleExternalId?: string;
-
-  /**
-   * Additional options to pass to STS when assuming the lookup role.
-   *
-   * - `RoleArn` should not be used. Use the dedicated `lookupRoleArn` property instead.
-   * - `ExternalId` should not be used. Use the dedicated `lookupRoleExternalId` instead.
-   *
-   * @see https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/STS.html#assumeRole-property
-   * @default - No additional options.
-   */
-  readonly assumeRoleAdditionalOptions?: { [key: string]: any };
-}
-
-/**
- * Query to AMI context provider
- */
-export interface AmiContextQuery extends ContextLookupRoleOptions {
   /**
    * Owners to DescribeImages call
    *
@@ -120,12 +97,46 @@ export interface AmiContextQuery extends ContextLookupRoleOptions {
 /**
  * Query to availability zone context provider
  */
-export interface AvailabilityZonesContextQuery extends ContextLookupRoleOptions {}
+export interface AvailabilityZonesContextQuery {
+  /**
+   * Query account
+   */
+  readonly account: string;
+
+  /**
+   * Query region
+   */
+  readonly region: string;
+
+  /**
+   * The ARN of the role that should be used to look up the missing values
+   *
+   * @default - None
+   */
+  readonly lookupRoleArn?: string;
+}
 
 /**
  * Query to hosted zone context provider
  */
-export interface HostedZoneContextQuery extends ContextLookupRoleOptions {
+export interface HostedZoneContextQuery {
+  /**
+   * Query account
+   */
+  readonly account: string;
+
+  /**
+   * Query region
+   */
+  readonly region: string;
+
+  /**
+   * The ARN of the role that should be used to look up the missing values
+   *
+   * @default - None
+   */
+  readonly lookupRoleArn?: string;
+
   /**
    * The domain name e.g. example.com to lookup
    */
@@ -152,7 +163,24 @@ export interface HostedZoneContextQuery extends ContextLookupRoleOptions {
 /**
  * Query to SSM Parameter Context Provider
  */
-export interface SSMParameterContextQuery extends ContextLookupRoleOptions {
+export interface SSMParameterContextQuery {
+  /**
+   * Query account
+   */
+  readonly account: string;
+
+  /**
+   * Query region
+   */
+  readonly region: string;
+
+  /**
+   * The ARN of the role that should be used to look up the missing values
+   *
+   * @default - None
+   */
+  readonly lookupRoleArn?: string;
+
   /**
    * Parameter name to query
    */
@@ -162,7 +190,24 @@ export interface SSMParameterContextQuery extends ContextLookupRoleOptions {
 /**
  * Query input for looking up a VPC
  */
-export interface VpcContextQuery extends ContextLookupRoleOptions {
+export interface VpcContextQuery {
+  /**
+   * Query account
+   */
+  readonly account: string;
+
+  /**
+   * Query region
+   */
+  readonly region: string;
+
+  /**
+   * The ARN of the role that should be used to look up the missing values
+   *
+   * @default - None
+   */
+  readonly lookupRoleArn?: string;
+
   /**
    * Filters to apply to the VPC
    *
@@ -204,7 +249,24 @@ export interface VpcContextQuery extends ContextLookupRoleOptions {
 /**
  * Query to endpoint service context provider
  */
-export interface EndpointServiceAvailabilityZonesContextQuery extends ContextLookupRoleOptions {
+export interface EndpointServiceAvailabilityZonesContextQuery {
+  /**
+   * Query account
+   */
+  readonly account: string;
+
+  /**
+   * Query region
+   */
+  readonly region: string;
+
+  /**
+   * The ARN of the role that should be used to look up the missing values
+   *
+   * @default - None
+   */
+  readonly lookupRoleArn?: string;
+
   /**
    * Query service name
    */
@@ -229,7 +291,7 @@ export enum LoadBalancerType {
 /**
  * Filters for selecting load balancers
  */
-export interface LoadBalancerFilter extends ContextLookupRoleOptions {
+export interface LoadBalancerFilter {
   /**
    * Filter load balancers by their type
    */
@@ -251,7 +313,24 @@ export interface LoadBalancerFilter extends ContextLookupRoleOptions {
 /**
  * Query input for looking up a load balancer
  */
-export interface LoadBalancerContextQuery extends LoadBalancerFilter {}
+export interface LoadBalancerContextQuery extends LoadBalancerFilter {
+  /**
+   * Query account
+   */
+  readonly account: string;
+
+  /**
+   * Query region
+   */
+  readonly region: string;
+
+  /**
+   * The ARN of the role that should be used to look up the missing values
+   *
+   * @default - None
+   */
+  readonly lookupRoleArn?: string;
+}
 
 /**
  * The protocol for connections from clients to the load balancer
@@ -292,6 +371,23 @@ export enum LoadBalancerListenerProtocol {
  * Query input for looking up a load balancer listener
  */
 export interface LoadBalancerListenerContextQuery extends LoadBalancerFilter {
+  /**
+   * Query account
+   */
+  readonly account: string;
+
+  /**
+   * Query region
+   */
+  readonly region: string;
+
+  /**
+   * The ARN of the role that should be used to look up the missing values
+   *
+   * @default - None
+   */
+  readonly lookupRoleArn?: string;
+
   /**
    * Find by listener's arn
    * @default - does not find by listener arn
@@ -314,7 +410,24 @@ export interface LoadBalancerListenerContextQuery extends LoadBalancerFilter {
 /**
  * Query input for looking up a security group
  */
-export interface SecurityGroupContextQuery extends ContextLookupRoleOptions {
+export interface SecurityGroupContextQuery {
+  /**
+   * Query account
+   */
+  readonly account: string;
+
+  /**
+   * Query region
+   */
+  readonly region: string;
+
+  /**
+   * The ARN of the role that should be used to look up the missing values
+   *
+   * @default - None
+   */
+  readonly lookupRoleArn?: string;
+
   /**
    * Security group id
    *
@@ -340,7 +453,24 @@ export interface SecurityGroupContextQuery extends ContextLookupRoleOptions {
 /**
  * Query input for looking up a KMS Key
  */
-export interface KeyContextQuery extends ContextLookupRoleOptions {
+export interface KeyContextQuery {
+  /**
+   * Query account
+   */
+  readonly account: string;
+
+  /**
+   * Query region
+   */
+  readonly region: string;
+
+  /**
+   * The ARN of the role that should be used to look up the missing values
+   *
+   * @default - None
+   */
+  readonly lookupRoleArn?: string;
+
   /**
    * Alias name used to search the Key
    */

diff --git a/lib/manifest.ts b/lib/manifest.ts
@@ -147,7 +147,11 @@ export class Manifest {
     return this.loadAssemblyManifest(filePath);
   }
 
-  private static validate(manifest: any, schema: jsonschema.Schema, options?: LoadManifestOptions) {
+  private static validate(
+    manifest: { version: string },
+    schema: jsonschema.Schema,
+    options?: LoadManifestOptions
+  ) {
     function parseVersion(version: string) {
       const ver = semver.valid(version);
       if (!ver) {
@@ -175,8 +179,7 @@ export class Manifest {
       nestedErrors: true,
 
       allowUnknownAttributes: false,
-      preValidateProperty: Manifest.validateAssumeRoleAdditionalOptions,
-    });
+    } as any);
 
     let errors = result.errors;
     if (options?.skipEnumCheck) {
@@ -247,34 +250,6 @@ export class Manifest {
     );
   }
 
-  /**
-   * Validates that `assumeRoleAdditionalOptions` doesn't contain nor `ExternalId` neither `RoleArn`, as they
-   * should have dedicated properties preceding this (e.g `assumeRoleArn` and `assumeRoleExternalId`).
-   */
-  private static validateAssumeRoleAdditionalOptions(
-    instance: any,
-    key: string,
-    _schema: jsonschema.Schema,
-    _options: jsonschema.Options,
-    _ctx: jsonschema.SchemaContext
-  ) {
-    if (key !== 'assumeRoleAdditionalOptions') {
-      // note that this means that if we happen to have a property named like this, but that
-      // does want to allow 'RoleArn' or 'ExternalId', this code will have to change to consider the full schema path.
-      // I decided to make this less granular for now on purpose because it fits our needs and avoids having messy
-      // validation logic due to various schema paths.
-      return;
-    }
-
-    const assumeRoleOptions = instance[key];
-    if (assumeRoleOptions?.RoleArn) {
-      throw new Error(`RoleArn is not allowed inside '${key}'`);
-    }
-    if (assumeRoleOptions?.ExternalId) {
-      throw new Error(`ExternalId is not allowed inside '${key}'`);
-    }
-  }
-
   /**
    * See explanation on `patchStackTagsOnRead`
    *