GoogleCloudPlatform / k8s-config-connector

GCP Config Connector, a Kubernetes add-on for managing GCP resources
https://cloud.google.com/config-connector/docs/overview
Apache License 2.0
902 stars 235 forks source link

Secret Manager Secret Version can't be updated #541

Open mikebryant opened 3 years ago

mikebryant commented 3 years ago

Checklist

Bug Description

SecretManagerSecretVersion doesn't handle updates

I wanted to use SecretManager to copy out a k8s secret that was being automatically generated (and updated) by cert-manager, so I used this config:

---
apiVersion: secretmanager.cnrm.cloud.google.com/v1beta1
kind: SecretManagerSecretVersion
metadata:
  name: istio-network-ca-cert
spec:
  enabled: true
  secretData:
    valueFrom:
      secretKeyRef:
        key: tls.crt
        name: istio-network-ca
  secretRef:
    name: istio-network-ca-cert

I was expecting this to create a new secret version whenever the secret data changed., but it fails

% kubectl describe secretmanagersecretversion istio-network-ca-cert | tail -n 16
Status:
  Conditions:
    Last Transition Time:  2021-08-25T16:41:21Z
    Message:               Update call failed: cannot make changes to immutable field(s): [secretData]
    Reason:                UpdateFailed
    Status:                False
    Type:                  Ready
  Create Time:             2021-08-25T10:11:10.520055Z
  Name:                    projects/350214784793/secrets/istio-network-ca-cert/versions/1
  Observed Generation:     1
Events:
  Type     Reason        Age                    From                                   Message
  ----     ------        ----                   ----                                   -------
  Warning  UpdateFailed  18m (x77 over 16h)     secretmanagersecretversion-controller  Update call failed: cannot make changes to immutable field(s): [secretData]
  Warning  UpdateFailed  10m (x13 over 10m)     secretmanagersecretversion-controller  Update call failed: cannot make changes to immutable field(s): [secretData]
  Warning  UpdateFailed  3m8s (x17 over 8m43s)  secretmanagersecretversion-controller  Update call failed: cannot make changes to immutable field(s): [secretData]

Additional Diagnostic Information

% k get secretmanagersecretversion -o yaml istio-network-ca-cert
apiVersion: secretmanager.cnrm.cloud.google.com/v1beta1
kind: SecretManagerSecretVersion
metadata:
  annotations:
    cnrm.cloud.google.com/management-conflict-prevention-policy: none
    cnrm.cloud.google.com/observed-secret-versions: '{"istio-network-ca":"574897"}'
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"secretmanager.cnrm.cloud.google.com/v1beta1","kind":"SecretManagerSecretVersion","metadata":{"annotations":{},"name":"istio-network-ca-cert","namespace":"default"},"spec":{"enabled":true,"secretData":{"valueFrom":{"secretKeyRef":{"key":"tls.crt","name":"istio-network-ca"}}},"secretRef":{"name":"istio-network-ca-cert"}}}
  creationTimestamp: "2021-08-25T10:11:08Z"
  finalizers:
  - cnrm.cloud.google.com/finalizer
  - cnrm.cloud.google.com/deletion-defender
  generation: 1
  managedFields:
  - apiVersion: secretmanager.cnrm.cloud.google.com/v1beta1
    fieldsType: FieldsV1
    fieldsV1:
      f:metadata:
        f:annotations:
          .: {}
          f:kubectl.kubernetes.io/last-applied-configuration: {}
      f:spec:
        .: {}
        f:enabled: {}
        f:secretData:
          .: {}
          f:valueFrom:
            .: {}
            f:secretKeyRef:
              .: {}
              f:key: {}
              f:name: {}
        f:secretRef:
          .: {}
          f:name: {}
    manager: kubectl-client-side-apply
    operation: Update
    time: "2021-08-25T10:11:08Z"
  - apiVersion: secretmanager.cnrm.cloud.google.com/v1beta1
    fieldsType: FieldsV1
    fieldsV1:
      f:metadata:
        f:annotations:
          f:cnrm.cloud.google.com/observed-secret-versions: {}
        f:finalizers:
          .: {}
          v:"cnrm.cloud.google.com/deletion-defender": {}
          v:"cnrm.cloud.google.com/finalizer": {}
      f:status:
        .: {}
        f:conditions: {}
        f:createTime: {}
        f:name: {}
        f:observedGeneration: {}
    manager: cnrm-controller-manager
    operation: Update
    time: "2021-08-25T10:11:11Z"
  name: istio-network-ca-cert
  namespace: default
  resourceVersion: "990582"
  uid: e8416071-16c9-41e8-b61f-de4575780156
spec:
  enabled: true
  secretData:
    valueFrom:
      secretKeyRef:
        key: tls.crt
        name: istio-network-ca
  secretRef:
    name: istio-network-ca-cert
status:
  conditions:
  - lastTransitionTime: "2021-08-25T16:41:21Z"
    message: 'Update call failed: cannot make changes to immutable field(s): [secretData]'
    reason: UpdateFailed
    status: "False"
    type: Ready
  createTime: "2021-08-25T10:11:10.520055Z"
  name: projects/350214784793/secrets/istio-network-ca-cert/versions/1
  observedGeneration: 1

Kubernetes Cluster Version

% kubectl version --short
Client Version: v1.20.4
Server Version: v1.20.8-gke.900

Config Connector Version

% kubectl get ns cnrm-system -o jsonpath='{.metadata.annotations.cnrm\.cloud\.google\.com/version}'
1.52.0%

Config Connector Mode

 kubectl get ConfigConnector "configconnector.core.cnrm.cloud.google.com" -o=jsonpath="{@.spec.mode}"
cluster%

Log Output

% k -n cnrm-system logs cnrm-controller-manager-0 manager | grep istio-network-ca-cert | tail -n 20
{"severity":"info","logger":"secretmanagersecretversion-controller","msg":"starting reconcile","resource":{"namespace":"default","name":"istio-network-ca-cert"}}
{"severity":"error","logger":"controller-runtime.manager.controller.secretmanagersecretversion-controller","msg":"Reconciler error","reconciler group":"secretmanager.cnrm.cloud.google.com","reconciler kind":"SecretManagerSecretVersion","name":"istio-network-ca-cert","namespace":"default","error":"Update call failed: cannot make changes to immutable field(s): [secretData]"}
{"severity":"info","logger":"secretmanagersecretversion-controller","msg":"starting reconcile","resource":{"namespace":"default","name":"istio-network-ca-cert"}}
{"severity":"error","logger":"controller-runtime.manager.controller.secretmanagersecretversion-controller","msg":"Reconciler error","reconciler group":"secretmanager.cnrm.cloud.google.com","reconciler kind":"SecretManagerSecretVersion","name":"istio-network-ca-cert","namespace":"default","error":"Update call failed: cannot make changes to immutable field(s): [secretData]"}
{"severity":"info","logger":"secretmanagersecretversion-controller","msg":"starting reconcile","resource":{"namespace":"default","name":"istio-network-ca-cert"}}
{"severity":"error","logger":"controller-runtime.manager.controller.secretmanagersecretversion-controller","msg":"Reconciler error","reconciler group":"secretmanager.cnrm.cloud.google.com","reconciler kind":"SecretManagerSecretVersion","name":"istio-network-ca-cert","namespace":"default","error":"Update call failed: cannot make changes to immutable field(s): [secretData]"}
{"severity":"info","logger":"secretmanagersecretversion-controller","msg":"starting reconcile","resource":{"namespace":"default","name":"istio-network-ca-cert"}}
{"severity":"error","logger":"controller-runtime.manager.controller.secretmanagersecretversion-controller","msg":"Reconciler error","reconciler group":"secretmanager.cnrm.cloud.google.com","reconciler kind":"SecretManagerSecretVersion","name":"istio-network-ca-cert","namespace":"default","error":"Update call failed: cannot make changes to immutable field(s): [secretData]"}
{"severity":"info","logger":"secretmanagersecretversion-controller","msg":"starting reconcile","resource":{"namespace":"default","name":"istio-network-ca-cert"}}
{"severity":"error","logger":"controller-runtime.manager.controller.secretmanagersecretversion-controller","msg":"Reconciler error","reconciler group":"secretmanager.cnrm.cloud.google.com","reconciler kind":"SecretManagerSecretVersion","name":"istio-network-ca-cert","namespace":"default","error":"Update call failed: cannot make changes to immutable field(s): [secretData]"}
{"severity":"info","logger":"secretmanagersecretversion-controller","msg":"starting reconcile","resource":{"namespace":"default","name":"istio-network-ca-cert"}}
{"severity":"error","logger":"controller-runtime.manager.controller.secretmanagersecretversion-controller","msg":"Reconciler error","reconciler group":"secretmanager.cnrm.cloud.google.com","reconciler kind":"SecretManagerSecretVersion","name":"istio-network-ca-cert","namespace":"default","error":"Update call failed: cannot make changes to immutable field(s): [secretData]"}
{"severity":"info","logger":"secretmanagersecret-controller","msg":"starting reconcile","resource":{"namespace":"default","name":"istio-network-ca-cert"}}
{"severity":"info","logger":"secretmanagersecret-controller","msg":"underlying resource already up to date","resource":{"namespace":"default","name":"istio-network-ca-cert"}}
{"severity":"info","logger":"secretmanagersecret-controller","msg":"successfully finished reconcile","resource":{"namespace":"default","name":"istio-network-ca-cert"}}
{"severity":"info","logger":"iampolicymember-controller","msg":"Starting reconcile","resource":{"namespace":"default","name":"istio-network-ca-cert"}}
{"severity":"info","logger":"iamclient","msg":"underlying resource is already up to date","resource":{"namespace":"default","name":"istio-network-ca-cert"}}
{"severity":"info","logger":"iampolicymember-controller","msg":"Finished reconcile","resource":{"namespace":"default","name":"istio-network-ca-cert"}}
{"severity":"info","logger":"secretmanagersecretversion-controller","msg":"starting reconcile","resource":{"namespace":"default","name":"istio-network-ca-cert"}}
{"severity":"error","logger":"controller-runtime.manager.controller.secretmanagersecretversion-controller","msg":"Reconciler error","reconciler group":"secretmanager.cnrm.cloud.google.com","reconciler kind":"SecretManagerSecretVersion","name":"istio-network-ca-cert","namespace":"default","error":"Update call failed: cannot make changes to immutable field(s): [secretData]"}

Steps to Reproduce

Steps to reproduce the issue

Create a SecretManagerSecretVersion based off a Secret, wait for it to create the version, then update the Secret

YAML snippets

---
apiVersion: secretmanager.cnrm.cloud.google.com/v1beta1
kind: SecretManagerSecret
metadata:
  name: istio-network-ca-cert
spec:
  replication:
    automatic: true
---
apiVersion: secretmanager.cnrm.cloud.google.com/v1beta1
kind: SecretManagerSecretVersion
metadata:
  name: istio-network-ca-cert
spec:
  enabled: true
  secretData:
    valueFrom:
      secretKeyRef:
        key: tls.crt
        name: istio-network-ca
  secretRef:
    name: istio-network-ca-cert
caieo commented 3 years ago

Hi @mikebryant, thank you for the detailed information & description of what you expect to happen. The way Config Connector works right now is that if there's a change in an immutable field, then ConfigConnector will return an error explaining that the field cannot be modified. In your scenario, the immutable field is the spec.secretData which is where the k8s secret is. Once you update the k8s secret, you will need to recreate a SecretManagerSecretVersion because you can't update the spec.secretData field. One possible workaround/solution for you might be to have a simple controller recreate the SecretManagerSecretVersion if the k8s secret is updated.

You mentioned that you were "expecting this to create a new secret version whenever the secret data changed", which might be a possible feature we can look into. I will bring this up to the team to consider this as an enhancement/feature request. In the meantime, can you provide us some more information for whether this is a nice-to-have, friction point, or blocker for you?

mikebryant commented 3 years ago

The way Config Connector works right now is that if there's a change in an immutable field, then ConfigConnector will return an error explaining that the field cannot be modified. In your scenario, the immutable field is the spec.secretData which is where the k8s secret is. Once you update the k8s secret, you will need to recreate a SecretManagerSecretVersion because you can't update the spec.secretData field.

This isn't quite accurate - I'm not updating the spec.secretData field. The content in the Secret that that field points to has been changed. The reference may be immutable - but Secrets themselves are not immutable.

It's also worth noting that the SecretManagerSecretVersion doesn't have a field specifying which secret version it's supposed to create/manage - which is another pointer to the expectation of it making sure the latest version matches the data.

One possible workaround/solution for you might be to have a simple controller recreate the SecretManagerSecretVersion if the k8s secret is updated.

Hmm. This sounds tricky... Would need to know if the version the SecretManagerSecretVersion is pointing to is actually up to date with the Secret contents. Which we don't have easy access to - that's the idea of using ConfigConnector here. And the idea of deleting / creating the object with exactly the same contents to make ConfigConnector wake up feels like a super weird API interaction.

You mentioned that you were "expecting this to create a new secret version whenever the secret data changed", which might be a possible feature we can look into. I will bring this up to the team to consider this as an enhancement/feature request.

Thank you :)

In the meantime, can you provide us some more information for whether this is a nice-to-have, friction point, or blocker for you?

I guess I would classify this as a big friction point for using ConfigConnector here. The workaround controller doesn't feel like a simple thing to write

caieo commented 3 years ago

@mikebryant, thanks for clarifying further! The current behavior of Config Connector with secrets is documented here. Even though the contents in the Secret changed but the reference didn't, that will still cause Config Connector to try and reconcile the new Secret, hence, the error you're running into.

After running it by the team, it doesn't seem like we will support this use-case in the near future. It looks like you might be able to accomplish a workaround by trying out Kpt with Config Sync, which should allow you to modify resource configurations and redeploy/recreate them as necessary.

sf-vorlov commented 1 year ago

Any update on this issue? Also are there any considerations to implement the equivalent of the "kubectl replace" (https://kubernetes.io/docs/reference/generated/kubectl/kubectl-commands#replace)? Which, for example, would allow for recreating the SecretManagerSecretVersion without the need for an additional controller

realsystem commented 1 month ago

Curious if any updates for the initial question. Looking for a ConfigConnector approach to manage items in GCP Secret Manager based on values generated by cert-manager in k8s.

Coder63 commented 1 month ago

@caieo : Any update / support for secretmanagersecretversion object natively with config connector ?!

maqiuyujoyce commented 1 month ago

@yuwenma FYI this looks like a feature request that we might be able to handle after migrating to direct controller.

yuwenma commented 1 month ago

Yes, we are working on this one right now and will share more updates soon.

w32-blaster commented 1 week ago

We also got into this exact same problem. Any estimate on when this will be available?

yuwenma commented 6 days ago

We also got into this exact same problem. Any estimate on when this will be available?

For sure, we are actively working on it, you can expect a change in the next 2-3 releases. Just to share a little bit more on what to expect: we are migrating the SecretManager Secret and SecretVersion resources to the direct approach, and making the entire SecretManager service more reliable and secure.