Add support for certificates/v1 API #1473

jrhouston · 2021-10-27T04:04:32Z

Description

This PR adds support for the certificates/v1 API by adding a new kubernetes_certificate_signing_request_v1 resource.

Acceptance tests

Have you added an acceptance test for the functionality being added?
Have you run the acceptance tests on this branch?

Output from acceptance testing:

=== RUN   TestAccKubernetesCertificateSigningRequestV1_basic
--- PASS: TestAccKubernetesCertificateSigningRequestV1_basic (3.34s)
=== RUN   TestAccKubernetesCertificateSigningRequestV1_generateName
--- PASS: TestAccKubernetesCertificateSigningRequestV1_generateName (3.31s)
PASS
ok  	github.com/hashicorp/terraform-provider-kubernetes/kubernetes	7.706s

Release Note

Release note for CHANGELOG:

Add support for certificates/v1 API

References

Community Note

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
If you are interested in working on this issue or have submitted a pull request, please leave a comment

jrhouston · 2021-10-27T04:09:18Z

kubernetes/resource_kubernetes_certificate_signing_request_v1.go

+						"signer_name": {
+							Type:        schema.TypeString,
+							Description: apiDocSpec["signerName"],
+							Required:    true,


This is now required by the API.

jrhouston · 2021-10-27T04:09:34Z

kubernetes/resource_kubernetes_certificate_signing_request_v1.go

+				return getErr
+			}
+			approval := certificates.CertificateSigningRequestCondition{
+				Status:  v1.ConditionTrue,


The status field is now required in the condition.

jrhouston · 2021-10-27T04:10:55Z

kubernetes/resource_kubernetes_certificate_signing_request_v1.go

+	}
+
+	log.Printf("[DEBUG] Waiting for certificate to be issued")
+	err = resource.RetryContext(ctx, d.Timeout(schema.TimeoutCreate), func() *resource.RetryError {


I changed this to use the RetryContext function rather than the StateChangeConf struct as we weren't actually dealing with any state here.

jrhouston · 2021-10-27T04:11:54Z

kubernetes/resource_kubernetes_certificate_signing_request_v1.go

+			log.Printf("[ERROR] Received error: %v", err)
+			return resource.NonRetryableError(err)
+		}
+


I removed some superfluous logic here, the for loop below is all that's required.

jrhouston · 2021-10-27T04:12:29Z

kubernetes/structures_certificate_signing_request_v1.go

+	if v, ok := in["usages"].(*schema.Set); ok && v.Len() > 0 {
+		obj.Usages = expandCertificateSigningRequestV1Usages(v.List())
+	}
+	if v, ok := in["signer_name"].(string); ok && v != "" {


I added signer_name here, as it was missing.

alexsomesan

Looks good, works in my testing.

sftim · 2021-11-25T18:55:59Z

The trouble with putting the API version into the Terraform code: if in future you want to replace a kubernetes_certificate_signing_request_v1 with a kubernetes_certificate_signing_request_v2 (because you want to, eg, turn off support for v1 CSRs in your cluster), you'll have to do some state file surgery to keep your existing resources in the Kubernetes API.

With cloud providers that provide multiple APIs for the same backing resource, Terraform typically doesn't expose details of which API Terraform is using. I'd expect to see a preference for any particular API configured at the provider level, not per-resource.

jrhouston · 2021-11-25T19:42:10Z

Hi @sftim thanks for sharing your thoughts. Fundamentally, I agree with what you're saying – I would love it were possible to specify the API group/version (or use the discovery client to check) and then have the provider use the appropriate schema for the resource when the provider plugin is loaded, either by specifying it in the provider block or even the resource itself in the same way that Kubernetes does.

Unfortunately down to the way the Terraform's protocol and type system works this isn't possible right now, and that kind of change is likely a major version away for Terraform if it's even feasible. The schema for the built-in resources (ones that aren't kubernetes_manifest) has to be known at build time, and there isn't a mechanism for resources to have multiple schema versions that can be built into the provider and toggled based on configuration. Certainly as Terraform evolves we will come back and revisit this.

So at present it's kind of a trade-off between:

Cobbling together frankenstein resources that combine the fields from multiple schemas to support different API versions, and have the provider figure out which API version needs to be used (this is what we do now)
Support only the one version of an API and have user rely on version pinning to get the API versions they need, and then backport fixes and changes to previous versions of the provider. This gets confusing for things like HPA where there is v1 but also v2 that's in beta, and potentially means including multiple kubernetes providers pinned at different versions.
Support distinct resources so we can have a cleanly defined schema for each API version, but rely on the user having to do some "state surgery" (i.e state rm, then import) if they need to move their resource to a different version.

I have written more about our motivation for doing this on this (unreleased) documentation page but basically the tl;dr is this is going to make it easier for us to use code generation to generate each resource in the provider from the Kubernetes API definition, rather than the currently laborious manual process it is today. The third option above (I feel) is the lesser of those 3 evils to help get us to where the lag time between changes the Kubernetes API landing as built-in resources in the provider is much shorter.

Hopefully that makes sense.

jrhouston · 2021-11-25T20:02:35Z

Terraform typically doesn't expose details of which API Terraform is using

Also, to your point on this, there is some prior art on this in the AWS provider where to support multiple versions of the underlying cloud API the version number actually is exposed in the resource name for a few services like CloudHSM and API Gateway.

Another option that occurred to me would be to actually break the provider down into multiple provider plugins for each API group (e.g having kubernetes-core, kubernetes-autoscaling etc.) and let the user select API versions that way. That's something to be explored after we tackle code generation though.

sftim · 2021-11-26T09:09:39Z

@jrhouston if there's a GitHub issue or discussion about this (in the terraform-provider-kubernetes, maybe I can point that out to some folks from the Kubernetes architecture folks and see if there's some options to recommend or explore?

I can start one but also very happy to let you take a lead there.

github-actions · 2021-12-27T02:10:20Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

github-actions bot added documentation size/XL labels Oct 27, 2021

jrhouston force-pushed the certificates-v1 branch from d6ae153 to 61c9b6a Compare October 27, 2021 04:08

jrhouston commented Oct 27, 2021

View reviewed changes

jrhouston requested a review from alexsomesan October 27, 2021 04:13

jrhouston force-pushed the certificates-v1 branch 2 times, most recently from a3d83c0 to fb94507 Compare November 2, 2021 02:51

Add support for certificates/v1 API

d07db75

jrhouston force-pushed the certificates-v1 branch from fb94507 to d07db75 Compare November 2, 2021 03:47

alexsomesan approved these changes Nov 4, 2021

View reviewed changes

jrhouston merged commit d07b044 into main Nov 9, 2021

jrhouston deleted the certificates-v1 branch November 9, 2021 04:12

curtbushko mentioned this pull request Nov 25, 2021

Add support for v1 APIs before Kubernetes version 1.22 is released #1049

Closed

github-actions bot locked as resolved and limited conversation to collaborators Dec 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for certificates/v1 API #1473

Add support for certificates/v1 API #1473

jrhouston commented Oct 27, 2021

jrhouston Oct 27, 2021

jrhouston Oct 27, 2021

jrhouston Oct 27, 2021

jrhouston Oct 27, 2021 •

edited

jrhouston Oct 27, 2021

alexsomesan left a comment

sftim commented Nov 25, 2021

jrhouston commented Nov 25, 2021 •

edited

jrhouston commented Nov 25, 2021

sftim commented Nov 26, 2021

github-actions bot commented Dec 27, 2021

Add support for certificates/v1 API #1473

Add support for certificates/v1 API #1473

Conversation

jrhouston commented Oct 27, 2021

Description

Acceptance tests

Release Note

References

Community Note

jrhouston Oct 27, 2021

Choose a reason for hiding this comment

jrhouston Oct 27, 2021

Choose a reason for hiding this comment

jrhouston Oct 27, 2021

Choose a reason for hiding this comment

jrhouston Oct 27, 2021 • edited

Choose a reason for hiding this comment

jrhouston Oct 27, 2021

Choose a reason for hiding this comment

alexsomesan left a comment

Choose a reason for hiding this comment

sftim commented Nov 25, 2021

jrhouston commented Nov 25, 2021 • edited

jrhouston commented Nov 25, 2021

sftim commented Nov 26, 2021

github-actions bot commented Dec 27, 2021

jrhouston Oct 27, 2021 •

edited

jrhouston commented Nov 25, 2021 •

edited