Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guidance on Post-Deployment tests which require identification in a resource-based policy #17

Open
scubbo opened this issue Mar 14, 2023 · 7 comments

Comments

@scubbo
Copy link

scubbo commented Mar 14, 2023

Is your feature request related to a problem? Please describe.
The current reference implementation adds post-deployment testing by calling addPost and passing a SoapUITest or JMeterTest, both of which extend CodeBuildStep and are defined without a scope or environment. This works well for test executions which only rely on calling an external Internet-available endpoint (which is passed into these TestStacks as a CfnOutput).

However, for test cases which require interacting with created resources directly (perhaps to assert on conditions that should not be visible via the public API), or even if the apiUrl was instead an ApiGatewayLambda (which enforces IAM authentication), we have a problem:

  • In order to set a resource-based policy permitting the CodeBuild Role to interact with a resource, the ARN of the Role must be known at the time of defining the resource
  • In order for the CodeBuild Role to have a known Arn, the Role must be manually created and passed to the CodeBuild Step (if we reference codeBuildStep?.role?.roleArn or codeBuildStep?.role?.roleName for a CodeBuild step without an explicitly passed Role, the value is undefined)
  • In order to manually create a Role, new Role(...) requires a scope - which is not available in the current setup of creating a CodeBuild step.

Describe the solution you'd like
An expansion of the reference pipeline to demonstrate how to execute tests that require granting resource-based permissions to the Role that will execute the tests (or, a standalone example of how to do so)

Describe alternatives you've considered

  • Only grant the CodeBuildStep Role AssumeRole privileges, and create specific Roles for it to assume in order to carry out testing
    • I suspect that this will be the preferred solution. Note, though, that unless the assumable roles grant Assumption privileges to all Roles in the account where the test execution role resides, we have recovered the same problem of "how can these Roles define the Roles that should be able to assume them if the test execution role has an undefined name?". The attack surface area here is probably a lot smaller, though.
    • If there are multiple assumable roles created for the Test Execution Role to assume, this abstraction will leak into the test code, which must "know about" which role to assume for which action.
  • Create the role in the context of the pipeline, and pass into the service stack and the testing step
    • Into the service stack to reference for resource-based policies; into the testing step to use in the CodeBuildStep.
    • This also implies that all testing roles will be in the same account, adding a minor irritation that their IDs and names will have to be differentiated.
  • Create the role in the context of the stage and pass out to the testing step
  • Avoid tests which require resource-based policies
    • This might well be possible, but seems a restrictive solution. Tests which only exercise the public interface of a system have their place, but some behaviours of a system (off the top of my head - those which rely on updating some "behind-the-scenes" value, or those which should trigger an asynchronous job) would be impractical to test purely via an external interface.

Additional context
Add any other context or screenshots about the feature request here.

@scubbo
Copy link
Author

scubbo commented Mar 14, 2023

In fact, I realize now that resource-based policy consideration is unnecessary - even granting IAM permissions to the step's Role with step.role?.attachInlinePolicy1 relies on a scope, since new Policy(...) requires a scope.

Footnotes

  1. I haven't actually tested if this even compiles when an explicit Role is not provided.

@cplee
Copy link
Contributor

cplee commented Mar 23, 2023

These additional IAM permissions should be accomplished through the rolePolicyStatements property in the CodeBuildStep. Would that work for you?

@scubbo
Copy link
Author

scubbo commented Mar 26, 2023

That's a good answer to my follow-on comment, but doesn't address situations where strict resource-based policies are required. If we just grant Identity-based permissions to the CodeBuildStep's role via rolePolicyStatements (rather than creating an explicit Role and passing it in to the CodeBuildStep), then codeBuildStep?.role?.roleArn will be undefined (at least, my testing suggests so - but I might have missed an alternative way of referring to this value!). This then means that there is no way to set a complementary resource-based policy granting access to the appropriate resource. Colloquially - "the Role will be allowed to invoke the Lambda, but the Lambda will not allow the Role to invoke it".

If you'll forgive some loose pseudo-code, here's a sketch of the "_Create the role in the context of the pipeline, and pass into the service stack and the testing step _" approach, which is currently my preferred approach:

class PipelineStack extends cdk.Stack {
  const synthStep = new ShellStep('Synth', {
    ... parameters to synthesize the infrastructure, and build the appCode and testCode
  });
  const testCodeOutput = synthStep.addOutputDirectory('the/directory/where/testCode/build/is/output');
  const pipeline = new CodePipeline(this, 'Pipeline', {
    synth: synthStep
  });

  addStageToPipeline(this, 'beta', pipeline, testCodeOutput);
  addStageToPipeline(this, 'gamma', pipeline, testCodeOutput);
  addStageToPipeline(this, 'load-testing', pipeline, testCodeOutput);
  ...

}

addStageToPipeline(
    scope: Construct,
    stageIdentifier: string, // A key to lookup per-stage configuration. In a real application, this would probably be an Enum
    pipeline: CodePipeline,
    testCodeOutput: FileSet): void {
  const stageConfig = lookupPerStageConfig(stageIdentifier);
  // As detailed above - explicitly creating the TestingRole is necessary in order to have an ARN to provide to resource-based policies for Constructs in the actual Stage
  const testingRole = new Role(scope, `${stageIdentifier}-TestingRole`, {
    ... _not_ including (all) the `inlinePolicies`, because we do not yet have the ARNs of the Resources created in the Stage
  }
  const pipelineStage = new PipelineStage(scope, `${stageIdentifier}-Stage`, stageConfig, testingRole);
  const stageDeployment = pipeline.addStage(pipelineStage);
  stageDeployment.addPost(new CodeBuildStep(`${stageIdentifier}-testing`, {
    ...
    role: testingRole
  });
}

class PipelineStage extends cdk.Stage {
  constructor(scope: Construct, id: string, config: PipelineStageConfig, testingRole: Role) {
    super(scope, id, config); // PipelineStageConfig must extend StageProps

    // In a real application, many resources would be created, and divided into Stacks - but that's needless complication to illustrate my point
    const function = new Function(this, 'Function', {
      ...
    });
    function.grantInvoke(testingRole);
    const iamPolicyStatementWhichAllowsTestingRoleToCallFunction = new PolicyStatement(...);
    testingRole.addToPolicy(iamPolicyStatementWhichAllowsTestingRoleToCallFunction);
  }
}

and an even-more-abbreviated pseudocode representation of how "Create the role in the context of the stage and pass out to the testing step" would look:

addStageToPipeline(
    scope: Construct,
    stageIdentifier: string,
    pipeline: CodePipeline,
    testCodeOutput: FileSet): void {
  const stageConfig = lookupPerStageConfig(stageIdentifier);

  const pipelineStage = new PipelineStage(scope, `${stageIdentifier}-Stage`, stageConfig, testingRole);
  const stageDeployment = pipeline.addStage(pipelineStage);
  stageDeployment.addPost(new CodeBuildStep(`${stageIdentifier}-testing`, {
    ...
    role: pipelineStage.testingRole
  });
}

class PipelineStage extends cdk.Stage {

  testingRole: Role

  constructor(scope: Construct, id: string, config: PipelineStageConfig, testingRole: Role) {
    super(scope, id, config); // PipelineStageConfig must extend StageProps

    this.testingRole = new Role(this, 'TestingRole', {
      ...
    });

    const function = new Function(this, 'Function', {
      ...
    });
    function.grantInvoke(this.testingRole);
    const iamPolicyStatementWhichAllowsTestingRoleToCallFunction = new PolicyStatement(...);
    this.testingRole.addToPolicy(iamPolicyStatementWhichAllowsTestingRoleToCallFunction);
  }
}

It's possible that the answer might simply be "there's no preferred method, do whichever makes most sense in the context of your application", and indeed I do recognize that these two approaches are extremely similar. Just wondering if AWS' significantly more-advanced practice in this area has highlighted any advantages or pitfalls of either.

@scubbo
Copy link
Author

scubbo commented Apr 18, 2023

I'm looking at this issue again - this time, because we're implementing per-developer personal stacks, which imitate the stacks deployed in Pipeline Stages, but are deployed (based on local versions of code) from terminal commands rather than from a Pipeline. This means that, if testingRoles are created in the context of a pipeline, then these personal stacks' testingRoles1 will need to be passed a reference to the pipeline environment itself. Maybe this is an argument in favour of "Create the role in the context of the stage and pass out to the testing step"?

I'm curious if there's been any more thought on this problem from your side.

Footnotes

  1. if the personal stacks aren't in a pipeline, why do they need testingRoles for executing integration tests? Because running integration tests against personal stacks before pushing the code is a good practice to help prevent unexpected integration test failures in the pipeline.

@cplee
Copy link
Contributor

cplee commented Apr 19, 2023

Hey @scubbo - I'm not a fan of introducing testing resources within the application stack. That puts me opposed to "Create the role in the context of the stage and pass out to the testing step".

I'd suggest a third option - a separate CDK stack that includes the testing resources (both the roles and the policies on the role allowing access to the function). This separate stack would take the lambda ARN as a parameter on the constructor. This allows the pipeline to deploy the stack but also allows developers to deploy those as personal stacks.

@scubbo
Copy link
Author

scubbo commented Apr 24, 2023

Interesting ok, thanks - I'll tinker with that and get back to you!

I'm assuming this means that there would be a Stack within the Stage, as a sibling to the Application Stacks, rather than a single TestingRole(s?)Stack deployed as an earlier Stage of the pipeline - that is, we're creating the Role in the context of the Stage, but not in the context of the ApplicationStack? Otherwise (if there was a single TestingRolesStack as a standalone stage of the pipeline), the cross-Stage (and cross-Environment) dependencies sound like they would be tricky to resolve!

@scubbo
Copy link
Author

scubbo commented Apr 27, 2023

Hmm, no, my assumption was incorrect - an attempt to create a Testing Role within a Stage, and then use that Testing Role in a CodeBuildStep in the pipeline, results in Error: You cannot have a dependency from 'PipelineStack' (in the App) to 'PipelineStack/prod-Stage/StackToHoldTestingRole' (in Stage 'PipelineStack/prod-Stage'): dependency cannot cross stage boundaries. So any role used in a CodeBuild step must not itself be declared in a Stage; so I guess the TestingRole's Stack will have to be instantiated alongside the Pipeline (and updated during the SelfUpdate step), but not explicitly part of (any stage of) the Pipeline. That still works!

Definitely looking forward to seeing the model solution updated with this - I've got a working and reasonably-satisfactory setup now, but I'm sure y'all can provide something a lot tidier! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants