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

Support docker-registry secret type #86

Closed
brantb opened this issue Apr 9, 2018 · 34 comments
Closed

Support docker-registry secret type #86

brantb opened this issue Apr 9, 2018 · 34 comments
Labels
Milestone

Comments

@brantb
Copy link

brantb commented Apr 9, 2018

I'm trying to seal a docker registry secret, but when the controller unseals the secret it is always of type: Opaque.

Sample:

$ kubectl create secret docker-registry my-registry \
  --docker-server=myserver.example.com \
  --docker-username=user \
  --docker-password=password \
  --docker-email=fake@example.com \
  --output json --dry-run | kubeseal > sealed.json
$ kubectl apply -f sealed.json
$ kubectl get secret my-registry -o yaml
apiVersion: v1
data:
  .dockerconfigjson: [snip]
kind: Secret
metadata:
  creationTimestamp: 2018-04-09T20:52:03Z
  name: my-registry
  namespace: default
  ownerReferences:
  - apiVersion: bitnami.com/v1alpha1
    controller: true
    kind: SealedSecret
    name: my-registry
    uid: db17d8cf-3c37-11e8-98fb-0a58ac1f010d
  resourceVersion: "3425531"
  selfLink: /api/v1/namespaces/default/secrets/my-registry
  uid: db1c68ac-3c37-11e8-98fb-0a58ac1f010d
type: Opaque

Expected result: The secret's type is kubernetes.io/dockerconfigjson.

Actual result: The secret type is Opaque.

@linoecarrillo
Copy link

@brantb You are able to seal docker-registry type secrets with v0.5.1. Haven't checked v0.6.0.

@anguslees anguslees added the bug label Apr 19, 2018
@anguslees
Copy link
Contributor

Oh! I bet this is a bug in the new 0.6.0 per-key encryption code, since now we look deeper inside the Secret object. I suggest using kubeseal from 0.5.1 for non-type=Opaque Secrets for now, since the 0.6.0 controller should fallback to the old scheme/code if given a pre-0.6.0 SealedSecret.

Hrm. Clearly, the integration test also needs to include a wider variety of Secrets now too...
@brantb Sorry about that, that's an embarrassing regression.

@jkinred
Copy link

jkinred commented Jul 9, 2018

Any chance we can get a release cut with the fix for this one?

We are running off quay.io/bitnami/sealed-secrets-controller:latest which feels a bit precarious.

@anguslees
Copy link
Contributor

anguslees commented Jul 9, 2018

Any chance we can get a release cut with the fix for this one?

This is stuck behind #92 - since the fix for that involves evolving the API type again from what was used to fix this issue (#88). I want to avoid releasing a version with just #88 since that will mean 2 transitions for affected users :/

I agree it's been a long time without a release that fixes this issue however :(

@pst
Copy link

pst commented Sep 15, 2018

This bug still affects version 0.7.0 so I would suggest to re-open this issue. Status closed, a merged PR and the fact that this is many months old is just wasting everyone's time. Make it clear that this is broken.

A workaround is to seal a type: kubernetes.io/dockerconfigjson secret with 0.5.1. The newer controllers unseal it just fine.

@anguslees
Copy link
Contributor

@pst is perfectly correct. Reopening until a new release is made. (If only github supported some sort of "Fixes" feature across more of the release cycle workflow...)

@anguslees anguslees reopened this Sep 17, 2018
@colt-jay
Copy link

colt-jay commented Nov 7, 2018

Confirmed the solution of reverting kubeseal to 0.5.1 to seal a docker secret works.

We also ran into this issue. Would be great if it could be addressed in the next release :)

@linuxbsdfreak
Copy link

Yes i am using the latest controller and the kubeseal only works with v0.5.1 for docker registry config

bors bot added a commit that referenced this issue Dec 18, 2018
135: Add a note about the secret type bug r=anguslees a=johnraz

I just got bitten by the bug in #86 and #92 and lost quite a bit of time finding out what was happening.
This will hopefully help other people figure it out faster until the proper fix is released.

Co-authored-by: Jonathan Liuti <liuti.john@gmail.com>
@spommerening
Copy link

Hmmmm, is there any way to find out which version of kubeseal I installed locally?
There seems to be no --version option for kubeseal or what am I doing wrong?

@karlskewes
Copy link
Contributor

@spommerening - current master has --version if you build it. Older releases don't have version flag.
You could re-download whichever version(s) you require.

@eimarfandino
Copy link

are you guys planning to resolve this soon in a future release?

@kraney
Copy link

kraney commented Mar 27, 2019

The suggested workaround does not work for me, with a 0.7.0 controller . I get this error in the logs:

Secret "artifactory-docker-secret" is invalid: type: Invalid value: "kubernetes.io/dockerconfigjson": field is immutable

@kraney
Copy link

kraney commented Mar 27, 2019

Never mind, deleting the sealedsecret and recreating it resolved the issue. (As compared to updating it.)

@radityasurya
Copy link

@kraney is it changing the type to Opaque or keeping the type to kubernetes.io/dockerconfigjson

@kraney
Copy link

kraney commented Mar 28, 2019

So with 0.7.0 secrets were transmuted from docker to Opaque.

After reading the workaround above I recreated the sealedsecret using 0.5.1. I’m using flux so I checked this in. Flux then applied the change as an update.

This resulted in the error you see above. Seemingly, sealedsecrets attempted to make the change as an update but since the ‘type’ field is immutable, this failed.

I then manually deleted the sealedsecret. A few minutes later flux put it back, this time as a create. Sealedsecrets did a corresponding create and everything started working.

It’s conceivably a separate bug that sealedsecrets attempts to apply a type change as an update. I don’t know if that’s something you’d hit, though, in absence of this bug. I supposed you might if a user made a mistake and then tried to correct it.

@kraney
Copy link

kraney commented Mar 28, 2019

One other thing I’d like to add is that IMO this bug should be considered higher priority than it seems to be getting. Even though there’s a workaround, it causes some pretty obtuse failures that can be quite hard to debug.

In my case, diagnosis was from an error message that a particular docker image wasn’t found. Not that there was a login failure with the registry, certainly not that the imagePullSecret was invalid. So this launched a long diagnosis.

It took a long time to notice that the secret that was correctly there and had the right data had the wrong type. The user experience of it is pretty bad.

@radityasurya
Copy link

Same thing, I reseal the secret with 0.5.1 and it works again, but the structure is really different.

@primeroz
Copy link

primeroz commented Apr 5, 2019

+1 for higher priority for this bug.

I keep hitting it for almost every use of sealed secrets

@ghost
Copy link

ghost commented May 21, 2019

Yep it's been over a year :(

@jaygorrell
Copy link

Ugh, also just ran into this but I see it's been a long time. Is this project still active?

@jaygorrell
Copy link

Just found the discussion in #165 that may be of interest to everyone on this issue.

@Bronek
Copy link

Bronek commented Jul 10, 2019

According to #170 (comment) this is now solved by #180

@demisx
Copy link

demisx commented Jul 17, 2019

It is indeed fixed on latest master. However, I had to manually edit the generated controller.yaml to change from:

image: sealed-secrets-controller:latest

to

image: quay.io/bitnami/sealed-secrets-controller:latest

so it doesn't try to pull the image from the Docker Hub. Is there a way to do generate controller.yaml with the correct image URL already in place?

@mkmik
Copy link
Collaborator

mkmik commented Jul 17, 2019

The CI run (see .travis.yml) sets the release image-name in CONTROLLER_IMAGE makefile variable. You can do it manually with:

$ make CONTROLLER_IMAGE=quay.io/bitnami/sealed-secrets-controller:latest controller.yaml

@mkmik
Copy link
Collaborator

mkmik commented Jul 17, 2019

Closing as the issue is now fixed; will make a new release soon

@mkmik mkmik closed this as completed Jul 17, 2019
@mkmik mkmik added this to the v0.9.0 milestone Jul 17, 2019
mkmik pushed a commit that referenced this issue Jul 17, 2019
Our CI system pushes the latest build to the `:latest` tag on quay.
It seems natural for users to desire that the controller.yaml file that gets built in master
points to such a build

(see #86 (comment))
bors bot added a commit that referenced this issue Jul 17, 2019
186: Use official docker image in main Makefile r=mkmik a=mkmik

Our CI system pushes the latest build to the `:latest` tag on quay.
It seems natural for users to desire that the controller.yaml file that gets built in master
points to such a build

(see #86 (comment))

Co-authored-by: Marko Mikulicic <mkm@bitnami.com>
@demisx
Copy link

demisx commented Jul 18, 2019

@mkmik Is there any way to install the latest master with the Helm chart? I'd like to deploy the fix to our cluster before the new release is published. Thank you very much.

@mkmik
Copy link
Collaborator

mkmik commented Jul 18, 2019 via email

@demisx
Copy link

demisx commented Jul 18, 2019

I'd just like to be able to bootstrap the cluster consistently using helm charts. I see there is one here for sealed secrets, but it currently points to v0.7 by default. I need to install what's the latest on master, because it fixes this bug. Is it just a matter of setting the image tag to latest?

helm install \
  --namespace kube-system \
  --name sealedsecrets \
  --set image.tag=latest \
  stable/sealed-secrets 

Sorry, if this is a dumb question - I am just starting to learn helm:

@demisx
Copy link

demisx commented Jul 18, 2019

FYI, when I tried it with --set image.tag=latest, the container fails to start. I see this:

Successfully assigned kube-system/sealedsecrets-sealed-secrets-568b4f785d-9t26j to ip-17...
Container image "quay.io/bitnami/sealed-secrets-controller:latest" already present on machine
Created container
Started container
Back-off restarting failed container

Is setting the image.tag not enough?

@mkmik
Copy link
Collaborator

mkmik commented Jul 19, 2019

The new controller.yaml file has changed quite a bit.
We'll do soon another release.

I assume the helm chart maintainer cannot update the chart until we cut a new release.

@mkmik mkmik modified the milestones: v0.9.0, v0.8.0 Jul 19, 2019
@demisx
Copy link

demisx commented Jul 19, 2019

@mkmik Thank you. That would be awesome! Many are so desperately waiting for v0.8.0. I will help with the chart release update, if needed, as well. This is such an excellent product and we are very happy to see it gaining some traction again.

@mkmik
Copy link
Collaborator

mkmik commented Jul 25, 2019

@demisx FWIW the helm chart for v0.8.0 has been released (helm/charts#15800)

@muhlba91
Copy link

I have tried using the chart including RBAC and received a startup error that the serviceaccount is not allowed to list secrets in the namespace the controller is deployed.

Is this an error in the helm chart RBAC with missing list permissions or an issue with the controller?

@mkmik
Copy link
Collaborator

mkmik commented Jul 25, 2019

@muhlba91
see helm/charts#15837

The helm chart hasn't been updated with the new yaml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests