Support github runners which bring their own AWS credentials

[christophgysin]

Currently there are two ways how to pass credentials:

• OIDC
• GitHub Secrets

In our current environment, we have runners backed by EC2 instances that already provide AWS credentials.

I would like to be able to choose neither of the above options, and let the workflow assume the runners already have credentials configured.

[christophgysin]

Would you be willing to merge a PR that implements this?

Currently the credential configurations are:

const pipeline = new GitHubWorkflow(app, 'Pipeline', {
  ...
  gitHubActionRoleArn: 'arn:aws:iam::<account-id>:role/GitHubActionRole',
}

const pipeline = new GitHubWorkflow(app, 'Pipeline', {
  ...
  awsCredentials: {
    accessKeyId: 'AWS_ACCESS_KEY_ID',
    secretAccessKey: 'AWS_SECRET_ACCESS_KEY',
  },
}

And if none is provided, we default to the latter.

I would suggest to simply not provide any credentials by default. That is a breaking change though, and I'm very happy to hear other suggestions, and implement them accordingly.

[kaizencc]

Hi @christophgysin! Sorry to hear that none of the current ways of passing credentials work for you. I'd like to learn a bit more about your use case -- right now, the github secrets get added in an awsCredentialsStep like so:

steps.push(awsCredentialStep('Authenticate Via GitHub Secrets', {
        region,
        accessKeyId: `\${{ secrets.${this.awsCredentials.accessKeyId} }}`,
        secretAccessKey: `\${{ secrets.${this.awsCredentials.secretAccessKey} }}`,
        ...(this.awsCredentials.sessionToken ? {
          sessionToken: `\${{ secrets.${this.awsCredentials.sessionToken} }}`,
        } : undefined),
        roleToAssume: assumeRoleArn,
      }));

Are you saying that in your case, you wish to omit the awsCredentialStep entirely? In terms of a solution, I don't think it makes sense to not provide credentials by default, because I can't imagine this being the "default" use case for most people. But there should be a (non-default) way to turn this off if you are already authenticated.

[christophgysin]

Hi @kaizencc! This is correct. We use bare EC2 instances as runners and would like to use the credentials provided by the instance role. I'm currently trying to patch out the credential steps using the JsonPatch escape hatches, but that is cumbersome and fragile.

I agree that this is most certainly not the default case, and can be hidden behind a non-default configuration option.

How about something like the following?

const pipeline = new GitHubWorkflow(app, 'Pipeline', {
  ...
  // This would also be the default if unset
  awsCredentials: AwsCredentals.fromGitHubSecrets({
    accessKeyId: 'AWS_ACCESS_KEY_ID',
    secretAccessKey: 'AWS_SECRET_ACCESS_KEY',
  }),
}

const pipeline = new GitHubWorkflow(app, 'Pipeline', {
  ...
  awsCredentials: AwsCredentals.fromOpenIdConnect({
    gitHubActionRoleArn: 'arn:aws:iam::<account-id>:role/GitHubActionRole',
  }),
}

const pipeline = new GitHubWorkflow(app, 'Pipeline', {
  ...
  awsCredentials: AwsCredentals.none(),
}

[kaizencc]

Hi @christophgysin, I'm not opposed to this at first glance. The tricky part (and why gitHubActionRoleArn and AwsCredentials are currently separate) is that the 2 ways of authenticating are completely different. In your proposal, what would the AwsCredentials class hold? In my mind, it might become a hodge-podge of different properties that are unique to the type of authentication.

We do have a way of sneakily hiding a feature behind a flag when we don't want to advertise it's existence. I wonder if that is an applicable solution here. Basically, I'm weary of exposing a way to provide no credentials -- because without your specific set up, that will fail, and cause confusion for users trying out the module. Here's what I'm thinking:

We do something hidden to turn off diff protection if you really have a reason for it:

    const diffProtection = this.node.tryGetContext('cdk-pipelines-github:diffProtection') ?? true;

Basically, if you set cdk-pipelines-github:diffProtection: false in cdk.json, then we don't run diff protection. I'm thinking we do something similar to support this feature, like cdk-pipelines-github:awsAuthentication: false, which if set, will skip the setUpAwsCredentials step.

Just a thought. I might grab a second opinion on this.

[christophgysin]

I would be happy with a secret flag to make my use case work for now.

[kaizencc]

Hi @christophgysin, apologies for going back on my previous suggestion -- I talked to some friends of the library (primarily because I'm not too familiar with your specific use case), and it makes sense to provide support for this in a more central way. The one caveat is that instead of AwsCredentials.none(), we should have AwsCredentials.runnerHasPreconfiguredCreds() -- that should assuage my concerns that none might be confusing.

I see the PR you've opened, which is a good start. If you're wiling, I think the enum-like class AwsCredentials you're suggesting makes sense. Would you like to modify your PR to support this?

If so, some thoughts:

• The PR will need tests
• We shouldn't remove gitHubActionRoleArn but simply deprecate it with the @deprecated flag. I know we're experimental so we technically can break things, but in this case I'd prefer if we didn't force everyone using OIDC to update their pipelines.

[christophgysin]

Sure, I can give it a try. How shall we handle the awsCredentials property? Changing the type would also introduce a breaking change, and if we want to deprecate it we need a different name for it. Any suggestions?

I would prefer the API defined in my comment above, with your suggested change:

const pipeline = new GitHubWorkflow(app, 'Pipeline', {
  ...
  awsCredentials: AwsCredentals.runnerHasPreconfiguredCreds(),
}

But I'm open to other suggestions.