feat: adding cert template for sub ca creation

[sepulworld]

Signed-off-by: Zane Williamson zwilliamson@flexport.com

• Adds support to request CA certs from aws-privateca-issuer.

[bmsiegel]

Thanks for submitting this pull request! Pull request reviews and check-ins are on-hold while we update testing. We will review pull requests in the order we received them once the update is complete. Please check back to view the status of the update and your pull request.

[MattLok]

Thanks for submitting this pull request! Pull request reviews and check-ins are on-hold while we update testing. We will review pull requests in the order we received them once the update is complete. Please check back to view the status of the update and your pull request.

Is there any estimated timeline on when the testing update will be completed?

[divyansh-gupta]

@MattLok The testing modifications are going through their final security review, after which we will review and accept new PRs. Thank you for your patience.

[anbaig]

Thanks for taking the time to cut this PR! Apologies for the delay on a review, just wanted to make sure I understood the implications of this PR before approving for merge. My largest concern here is that this is a one way door in terms of the use of the isCA flag. If this PR got merged as is, we would be forever committed to supporting the acm-pca:::template/SubordinateCACertificate_PathLen0/V1 template when that flag was toggled. It doesn’t leave room for the use of other CA templates, such as arn:aws:acm-pca:::template/SubordinateCACertificate_PathLen1/V1 or the other path length/passthrough templates. I’d be more willing to approve this for merge if we could address this issue. Some ideas I had:

• Perhaps we could make aws-privateca-issuers configurable with a flag TemplateArn that if set would make it so that any certificate issued off that issuer uses that template?
• Alternatively, we would add a flag that is called SubCAPathLen, which is required to be set if the isCA flag is toggled?

Thoughts?

[sepulworld]

This did come to mind when putting this pull request in. However, there are presently no configuration interfaces for this plugin ("isCA" is part of the cert-manager object that is passed to plugin). This means we will have to introduce something new to the plugins design that is a bit outside of intended scope. (which I am fine with!).

I think option 2 sounds good. Ill try to find time to add the api interface and update this

[sepulworld]

Can we add a new spec input without having to go to cert-manager to update interface there?

for example adding path length as an input...

apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
  name: pca-issuer-rsa-ca-cert
  namespace: default
spec:
  isCA: true
  subCAPathLen: 1
---snip---

Let me try it out and see if it works for us

[anbaig]

Can we add a new spec input without having to go to cert-manager to update interface there?

for example adding path length as an input...

apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
  name: pca-issuer-rsa-ca-cert
  namespace: default
spec:
  isCA: true
  subCAPathLen: 1
---snip---

Let me try it out and see if it works for us

I don't think you can update the Certificate Kind without cutting a PR to Cert-Manager. The creation of the Certificate will kick off the Cert-Manager controller which will try to generate a CSR but will probably run into issues when it sees "subCAPathLen" and doesn't understand what that is.

[sepulworld]

without

Ok, any suggestions on how we should introduce variable inputs for this issuer?

[sepulworld]

without

Ok, any suggestions on how we should introduce variable inputs for this issuer?

I just reread your initial suggestion. By adding a flag you are referring to a flag for the process (https://github.com/cert-manager/aws-privateca-issuer/blob/master/main.go#L54) where we can define the pathLength. Let me take a look into this.

[anbaig]

When I meant adding a flag, I meant introducing a new attribute into the AWSPCAIssuer/ClusterIssuer kind. Sort of like how it was done in this PR: https://github.com/cert-manager/aws-privateca-issuer/pull/42/files.

[diranged]

So - just to be clear the concern here is that the current code (ultimately a VERY simple change) is not flexible enough to allow creating a SubCA that can then create more SubCAs under it, right? This is something that AWS barely supports on their own right now, and in fact prohibits you from doing across accounts through the Resource Access Manager service anyways.

Can we get this merged as-is for now? If you really want it to be configurable, make it a flag that is set on the aws-privateca pod at runtime.. sort of a cluster-level setting?

@anbaig Do you have a built-docker-image of this patch that we can test out?

[diranged]

@anbaig So I was able to build the customized build with diranged@09c2060 and prove that this works, and works well. I strongly suggest finishing the PR (removing the check) and getting this out.. it's great.

[anbaig]

@diranged thanks for bringing up the issue about the sub CA template that came out of the discussion not being supported by cross account. I went ahead and made the default the Path Len zero template that is not the blank or APICSR passthrough variant. I also went ahead and added relevant unit tests and integ test to the branch @sepulworld!

[diranged]

@diranged thanks for bringing up the issue about the sub CA template that came out of the discussion not being supported by cross account. I went ahead and made the default the Path Len zero template that is not the blank or APICSR passthrough variant. I also went ahead and added relevant unit tests and integ test to the branch @sepulworld!

Thank you!

[jetstack-bot]

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: divyansh-gupta, sepulworld

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

The pull request process is described 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