Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move certificate id to status for ComputeManagedSSLCert #900

Merged

Conversation

diviner524
Copy link
Collaborator

Change description

It seems the TF provider mistakenly marked this field as optional thus ended up as a spec field in Config Connector CRD.

The REST API doc also confirms this field should be output only.

https://cloud.google.com/compute/docs/reference/rest/v1/regionSslCertificates#insert

"[Output Only] The unique identifier for the resource. This identifier is defined by the server."

Created a patch to correct this.

Fixes #107 b/296394223

Tests you have done

Tested in my dev cluster, and the ComputeManagedSSLCert is now UpToDate.

apiVersion: compute.cnrm.cloud.google.com/v1alpha1
kind: ComputeManagedSSLCertificate
metadata:
  name: mcert01
spec:
  managed:
    domains:
    - sslcert.tf-test.club.
  projectRef:
    external: <test-project-id>
  resourceID: mcert01
kubectl get -f cert.yaml 
NAME      AGE    READY   STATUS     STATUS AGE
mcert01   3d3h   True    UpToDate   3d3h
  • Run make ready-pr to ensure this PR is ready for review.
  • Perform necessary E2E testing for changed resources.

@maqiuyujoyce
Copy link
Collaborator

Hi @diviner524 thank you for the change! I believe previously we agreed on following http://go/kcc-breaking-changes#how-to-deal-with-breaking-changes for any breaking changes like this. Or did we decide to address it differently?

@diviner524
Copy link
Collaborator Author

Hi @diviner524 thank you for the change! I believe previously we agreed on following http://go/kcc-breaking-changes#how-to-deal-with-breaking-changes for any breaking changes like this. Or did we decide to address it differently?

It's a v1alpha1 resource, so I believe it is fine?

@maqiuyujoyce
Copy link
Collaborator

Ah gotcha! Yes, it's fine to have breaking changes in v1alpha1 CRDs. LGTM!

@google-oss-prow
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: diviner524, maqiuyujoyce

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 [diviner524,maqiuyujoyce]

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

@google-oss-prow google-oss-prow bot merged commit db79671 into GoogleCloudPlatform:master Oct 11, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Google-managed SSL certificates
2 participants