Add assume role for Issuer

[paragor]

Issue # 361

Closes # 361

Reason for this change

AWS RAM has restrictions that disallow issuing isCA: true certs from Subordinate AWS PCA.
see https://docs.aws.amazon.com/privateca/latest/userguide/UsingTemplates.html

However, for example, Linkerd requires such a certificate.

This pull request allows for requesting an issue certificate directly from AWS PCA that is located in a different account.

Description of changes

A new optional field 'role' was added to Issuer and ClusterIssuer CRDs, which allows assuming a role for another account and working with AWS PCA from a different account.

[cert-manager-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign sgtcodfish for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[nickperry]

@paragor I think you might need to rebase now, since #430 was merged.

[paragor]

@nickperry Hi, thank you for moving this topic forward. I’ll test my PR this weekend!

[bmsiegel]

Just pushed a rebase to this PR. We will need to work on writing integration tests as well for this change since it's going to end up being a new support auth flow.

[paragor]

I’ve just renamed the attribute role -> roleArn. I have tested it locally, and everything looks good.
I’ll try to finish writing the integration tests this week. However, I’ll need help running them - the test run is expensive because it requires creating several AWS PCAs.

If anyone would like to pick up and complete the tests, feel free to do so.

[bmsiegel]

I hate to do this, but can you change the role field back to just role? Cert-manager uses a role arn in one of its specs so we'd like to stay consistent with it:

apiVersion: cert-manager.io/v1
kind: Issuer
metadata:
  name: example
spec:
  acme:
    ...
    solvers:
    - dns01:
        route53:
          region: us-east-1
          role: <iam-role-arn> # This must be set so cert-manager what role to attempt to authenticate with
          auth:
            kubernetes:
              serviceAccountRef:
                name: <service-account-name> # The name of the service account created

source: https://cert-manager.io/docs/configuration/acme/dns01/route53/#cross-account-access

[nickperry]

I hate to do this, but can you change the role field back to just role?

Not my PR so I can't push to it, but I have implemented the name change requested by @bmsiegel here if you want to cherry-pick it nickperry@a670b1c

[ARichman555]

instead of hard coding the name, can we make it default to "IssuerTestRole" but can be overridden by an environment variable? Alternatively, we could just take the whole role arn in as an environment variable

[bmsiegel]

I wanted to do it with an environment variable, but then we couldn't run the testing before we merge it. What are your thoughts?

[bmsiegel]

Are you suggesting that we use an override and default to IssuerTestRole?

[ARichman555]

Yea, default to IssuerTestRole but then allow you to override it for more flexibility

[ARichman555]

I don't think it makes sense to enforce this regex

[bmsiegel]

Why not? If the role is formatted incorrectly and we can't find the name, what should we do?

[ARichman555]

From my understanding, this regex just enforces the format of RoleName-domain-region. If we generalize it to just accept any role name, we don't need this regex and can remove the domain aspect right?

[bmsiegel]

If we pair this with your other comment we can remove it. I was trying to make it domain safe. But this code was intended to pull it out for the tests