cdk-pipelines-github icon indicating copy to clipboard operation
cdk-pipelines-github copied to clipboard

`diffProtection` does not work when stack contains tokens

Open danieljamesscott opened this issue 3 years ago • 8 comments

The line: https://github.com/cdklabs/cdk-pipelines-github/blob/main/src/pipeline.ts#L300 does not seem to work when the stack contains tokens.

For example. I create the GitHub OIDC role in an app in my root CDK template, and pass this in to my GitHubWorkflow.gitHubActionRoleArn variable. When running the cdk synth in GHA, I get an error:

Please commit the updated workflow file

Which is caused because the workflow appears different because the tokens have changed:

-          role-to-assume: ${Token[TOKEN.227]}
+          role-to-assume: ${Token[TOKEN.234]}

This happens even though the template generates properly in the actual workflow file.

Is there a way to fix this? (Apart from setting "cdk-pipelines-github:diffProtection": false in my cdk.json)

danieljamesscott avatar Aug 15 '22 15:08 danieljamesscott

Oh no... thanks for the report @danieljamesscott. Let me think about what to do here. You're right, turning off diff protection might unblock you but is not the acceptable solution here.

kaizencc avatar Aug 15 '22 17:08 kaizencc

I think I spoke to soon. What you've described won't work, the GitHub OIDC role must be created separate to the pipeline and its arn must be passed in after it has been created. There should be no tokens in your deploy.yml file. Furthermore, only one GitHub OIDC role can be created per account, so if you already have it you have to reference the arn of that one, not create a new one.

All of this is described in the documentation, which says to create the GitHub OIDC role separate from the GitHub Pipeline, as a one-off cdk deploy to easily set up the role.

kaizencc avatar Aug 15 '22 18:08 kaizencc

the GitHub OIDC role must be created separate to the pipeline and its arn must be passed in after it has been created.

Please can you explain why this is? Is it purely to avoid tokens in the pipeline config? I have a role stack at the top of my pipeline config file which I deploy to bootstrap.

Furthermore, only one GitHub OIDC role can be created per account, so if you already have it you have to reference the arn of that one, not create a new one.

This is not an AWS constraint. Is it a constraint of cdk-pipelines-github? Please can you explain why?

All of this is described in the documentation, which says to create the GitHub OIDC role separate from the GitHub Pipeline, as a one-off cdk deploy to easily set up the role.

Yes, that is what I am doing. But I guess that it's not possible to create the stack in the same app as the pipeline, although I do not understand why.

danieljamesscott avatar Aug 15 '22 20:08 danieljamesscott

the GitHub OIDC role must be created separate to the pipeline and its arn must be passed in after it has been created.

Please can you explain why this is? Is it purely to avoid tokens in the pipeline config? I have a role stack at the top of my pipeline config file which I deploy to bootstrap.

Furthermore, only one GitHub OIDC role can be created per account, so if you already have it you have to reference the arn of that one, not create a new one.

This is not an AWS constraint. Is it a constraint of cdk-pipelines-github? Please can you explain why?

All of this is described in the documentation, which says to create the GitHub OIDC role separate from the GitHub Pipeline, as a one-off cdk deploy to easily set up the role.

Yes, that is what I am doing. But I guess that it's not possible to create the stack in the same app as the pipeline, although I do not understand why.

Oh, I guess this may be possible if I use an explicit CfnOutput and export it, then import it in the pipeline config?

danieljamesscott avatar Aug 15 '22 20:08 danieljamesscott

There should be no tokens in your deploy.yml file.

To clarify, there are no tokens in my locally synthesized deploy.yml file - that is working fine. The tokens only show up when running in GHA to check that the pipeline config has not been modified.

danieljamesscott avatar Aug 15 '22 20:08 danieljamesscott

Furthermore, only one GitHub OIDC role can be created per account, so if you already have it you have to reference the arn of that one, not create a new one.

This is not an AWS constraint. Is it a constraint of cdk-pipelines-github? Please can you explain why?

Sorry. the identity provider is what I'm talking about here. You can create multiple roles, sure, but you'd have to supply the identity provider into the role each time.


I think I know what's happening here. The big question is how is your GitHub Workflow going to get the value of the OIDC role arn? Locally, you can resolve your token by running cdk deploy on the stack with the role. The resulting arn can be sent to your GitHub Workflow, and the app with your workflow will not complain.

In GHA, it's not possible to run cdk deploy on your role stack to get the actual arn value prior to synthesizing the workflow. GHA needs the role in order to authenticate to AWS and allow a deployment to happen. So now you have a circular dependency of needing the role arn to deploy -> needing to deploy to get the role arn. So the diff protection is helping you here -- it is alerting you to its inability to resolve the token, and telling you that the workflow would fail if actually deployed.

kaizencc avatar Aug 15 '22 21:08 kaizencc

I think I know what's happening here. The big question is how is your GitHub Workflow going to get the value of the OIDC role arn? Locally, you can resolve your token by running cdk deploy on the stack with the role. The resulting arn can be sent to your GitHub Workflow, and the app with your workflow will not complain.

The workflow gets the role arn from deploy.yml, it's already in there.

In GHA, it's not possible to run cdk deploy on your role stack to get the actual arn value prior to synthesizing the workflow. GHA needs the role in order to authenticate to AWS and allow a deployment to happen. So now you have a circular dependency of needing the role arn to deploy -> needing to deploy to get the role arn. So the diff protection is helping you here -- it is alerting you to its inability to resolve the token, and telling you that the workflow would fail if actually deployed.

Not quite. I have already bootstrapped the pipeline (like we do for cdk) by running a cdk deploy locally to deploy the role, so it already exists and the arn is in deploy.yml for the workflow to use. This may be an issue with cdk itself. I'm not sure where the 'original' token values come from. Perhaps this is because we have not yet obtained the role credentials when attempting to run cdk synth? That part only happens later. I think that CDK solves this by creating outputs/parameters to allow roles to be discovered, maybe I can do something similar but it would be great if this was built in.

danieljamesscott avatar Aug 15 '22 22:08 danieljamesscott

+1 would appreciate being able to use tokens in the workflow. Is there some way for diff protection to ignore tokens? They're a fairly useful CDK feature, would be nice to have the same functionality

clairemr avatar Aug 22 '22 06:08 clairemr