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

controller does not reconcile Passwords and Secrets #91

Open
hoegaarden opened this issue Jul 18, 2022 · 1 comment
Open

controller does not reconcile Passwords and Secrets #91

hoegaarden opened this issue Jul 18, 2022 · 1 comment
Labels
bug This issue describes a defect or unexpected behavior carvel-accepted This issue should be considered for future work and that the triage process has been completed priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.

Comments

@hoegaarden
Copy link

hoegaarden commented Jul 18, 2022

What steps did you take:

I created a Password with some labels. Then I wanted to add another label and was hoping that this change gets pushed down to the Secret. Because this did not happen, I deleted the Secret in the hopes that the secretgen-controller would reconcile and recreate the Secret, now with the updated set of labels from the Password:

Steps taken:

  • install the secret-gen controller:
    k apply -f https://github.com/vmware-tanzu/carvel-secretgen-controller/releases/download/v0.9.1/release.yml
  • create a password
    cat <<'EOF' | k apply -f -
    kind: Password
    apiVersion: secretgen.k14s.io/v1alpha1
    metadata:
      name: the-passoword
      labels:
        initalLabel: something
    spec:
      length: 10
      secretTemplate:
        type: Secret
        stringData:
          password: $(value)
    EOF
  • inspect the labels on the Password and the Secret:
    k get password,secret -o yaml | yq -r '.items[] | "\(.kind): \(.metadata.name) => \(.metadata.labels)"'
    
    Password: the-passoword => {"initalLabel":"something"}
    Secret: the-passoword => {"initalLabel":"something"}
  • change labels on the Password:
    k label password/the-passoword anotherLabel=somethingElse
    
    password.secretgen.k14s.io/the-passoword labeled
  • inspect the labels again:
     k get password,secret -o yaml | yq -r '.items[] | "\(.kind): \(.metadata.name) => \(.metadata.labels)"'
    
    Password: the-passoword => {"anotherLabel":"somethingElse","initalLabel":"something"}
    Secret: the-passoword => {"initalLabel":"something"}
  • delete the secret:
    k delete secret the-passoword
  • see, that the secret is gone:
    k get password,secret -o yaml | yq -r '.items[] | "\(.kind): \(.metadata.name) => \(.metadata.labels)"'
    
    Password: the-passoword => {"anotherLabel":"somethingElse","initalLabel":"something"}
    # Note, we don't see no `Secret` anymore.
  • see, that there are no reconcile errors or something:
    k get password/the-passoword -o yaml | yq -y .status
    
    conditions:
    - status: 'True'
      type: ReconcileSucceeded
    friendlyDescription: Reconcile succeeded
    observedGeneration: 1

What happened:

"Nothing". First the Password did not update the labels on the Secret it created. And then, after the Secret was deleted, the controller made not attempts to recreate the Secret again.

What did you expect:

I expected

  • the controller to update the Secret's metadata when I update the Password's metadata
  • the controller to recreate the Secret or give me an error and tell me it cannot recreate the secret (Because it does not know the original password, which cloud be a valid failure case IMHO)

Anything else you would like to add:

I could see that there are reasons to not reconcile the Secret, because the controller has no knowledge of the original password, and for some cases it might be better to not reconcile than to reconcile with a changed password. However, if that's the case I think we should at least call that out in the docs or, ideally, have the controller tell users about that in its status subresource.
I think it is "surprising" that the controller seems to be doing nothing at all once the secret is created.

This bug report outlines two different issues. Technically, I guess, they need to address different things (watching updates/deletion of secrets vs. watching updated of passwords). I am happy to split the issue. However, from a user's point of view, I think they are "the same" in the sense that the controller does not seem to reconcile at all.

There was a short discussion about this in #carvel.

Environment:

  • secretgen-controller version
    k get deployment -n secretgen-controller secretgen-controller -o yaml | yq -r '.metadata.annotations["kbld.k14s.io/images"]'
    
    - origins:
      - local:
          path: /Users/dk/workspace/carvel-secretgen-controller
      - git:
          dirty: false
          remoteURL: git@github.com:vmware-tanzu/carvel-secretgen-controller
          sha: 3b76cf0602b6c377fca8589c5fa30b9f04fc51a0
          tags:
          - v0.9.1
      url: ghcr.io/vmware-tanzu/carvel-secretgen-controller@sha256:1f842b9c3e0ee164b450c10486d7b08eee0eacb754af42549aa64906221f7966
    
  • Kubernetes version (use kubectl version)
    k version
    WARNING: version difference between client (1.21) and server (1.24) exceeds the supported minor version skew of +/-1
    Client Version: version.Info{Major:"1", Minor:"21", GitVersion:"v1.21.1", GitCommit:"5e58841cce77d4bc13713ad2b91fa0d961e69192", GitTreeState:"clean", BuildDate:"2021-05-12T14:18:45Z", GoVersion:"go1.16.4", Compiler:"gc", Platform:"linux/amd64"}
    Server Version: version.Info{Major:"1", Minor:"24", GitVersion:"v1.24.0", GitCommit:"4ce5a8954017644c5420bae81d72b09b735c21f0", GitTreeState:"clean", BuildDate:"2022-05-19T15:39:43Z", GoVersion:"go1.18.1", Compiler:"gc", Platform:"linux/amd64"}

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

@hoegaarden hoegaarden added bug This issue describes a defect or unexpected behavior carvel-triage This issue has not yet been reviewed for validity labels Jul 18, 2022
@joe-kimmel-vmw
Copy link
Contributor

thanks @hoegaarden -- per the discussion in slack it sounds like this work is pre-approved but not yet scheduled:

i do agree that's not a nice behaviour. i would recommend filing a single issue for generators to regenerate if they see k8s Secret missing.

@joe-kimmel-vmw joe-kimmel-vmw added carvel-accepted This issue should be considered for future work and that the triage process has been completed priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed carvel-triage This issue has not yet been reviewed for validity labels Jul 20, 2022
@aaronshurley aaronshurley moved this to To Triage in Carvel Jul 25, 2022
@github-project-automation github-project-automation bot moved this to To Triage in Carvel Feb 14, 2023
@neil-hickey neil-hickey moved this from To Triage to Unprioritized in Carvel Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue describes a defect or unexpected behavior carvel-accepted This issue should be considered for future work and that the triage process has been completed priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
Status: Unprioritized
Development

No branches or pull requests

2 participants