Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Flux uses wrong imagePullSecret #1485

Closed
hiddeco opened this issue Oct 31, 2018 · 5 comments · Fixed by #1702
Closed

Flux uses wrong imagePullSecret #1485

hiddeco opened this issue Oct 31, 2018 · 5 comments · Fixed by #1702
Labels

Comments

@hiddeco
Copy link
Member

hiddeco commented Oct 31, 2018

Our clusters are running on GKE under GCP project-x. Images are however published to GCP projects dedicated to the product (e.g. GCP project product-y).

We add a Docker secret for this GCP project to every product's namespace and use this to pull from the GCR dedicated to this product with a imagePullSecrets set on the workload.

Now what Flux tries to do is to pull an image from product-y with the credentials from product-z.

Say we have a product subscription and a product publish Flux logs the following:

ts=2018-10-30T11:10:19.340162325Z caller=warming.go:192 component=warmer canonical_name=eu.gcr.io/subscription/image auth="{map[eu.gcr.io:<registry creds for [email protected], from publish:secret/gcrsecret>]}" err="requesting tags: denied: Failed to read tags for host 'eu.gcr.io', repository '/v2/subscription/image/tags/list'


After some thinkering with @squaremo we came to the conclusion that credential merge happens on registry (e.g. eu.gcr.io), which is the same for both secrets although they differ (they have a unique project_id).

I think the solution would be for Flux to adhere to the same rules as Kubernetes and only use imagePullSecrets from the same namespace as the workload.

@squaremo
Copy link
Member

squaremo commented Nov 1, 2018

I think the solution would be for Flux to adhere to the same rules as Kubernetes and only use imagePullSecrets from the same namespace as the workload.

Fluxd does only look at secrets in the same namespace as the workload (because those are the only secrets that are relevant to a workload). But the code in question is compiling credentials for images, not for workloads.

This is more or less the algorithm:

for each workload:
  merge all the secrets associated with it, as Kubernetes would do
  enter the merged credentials against each image used by the workload

Two things you may notice:

  • it's possible that the credentials for a given image will be replaced, by those from another workload; but,
  • each image will end up with credentials from a workload that uses that image

In other words, assuming that all the workloads run correctly, every image should have valid credentials.

Since we have documentary evidence of that not being the case, we have to reject one of the assumptions. Either

  1. the code doesn't work as described
  2. the code does work like that, but my conclusion is wrong
  3. the workloads in the cluster aren't configured correctly

My money is on 1. (though it might take a bit of test case construction and debugging to figure out how it's wrong).

@hiddeco
Copy link
Member Author

hiddeco commented Nov 1, 2018

I do not think option 3 is possible because with an incorrect configuration Kubernetes would not be able to pull the images either and each namespace only has one kubernetes.io/dockerconfigjson secret (for now).

Do you have a link to the described algorithm so I can take a look at it? I am also betting on 1.

@squaremo
Copy link
Member

squaremo commented Nov 1, 2018

Do you have a link to the described algorithm so I can take a look at it?

It's in cluster/kubernetes/images.go

@hiddeco
Copy link
Member Author

hiddeco commented Nov 7, 2018

After reading the description of your algorithm again and skimming trough images.go I think that I figured out the problem.

for each workload:
  merge all the secrets associated with it, as Kubernetes would do
  >> enter the merged credentials against each image used by the workload

The issue is in the last sentence. When two Pods with different imagePullSecrets share a sidecar with the same image from e.g. Docker Hub the pull secret from this Pod is stored against the imageID [1].

Later in the process (warmer.go) these credentials end up getting loaded in to the client. Because they share the same registry domain (eu.gcr.io) the client attempts to retrieve the image tags with the wrong credentials (I think).


You can validate my theory by following the next steps:

  1. create two namespaces with different Docker credentials
  2. create two deployments that both retrieve different images that can only be pulled with the credentials from the namespace
  3. watch the logs for errors (there are none)
  4. modify the deployments and add a sidecar with an image from Docker Hub
  5. watch the logs (errors will surface)

@squaremo
Copy link
Member

squaremo commented Nov 8, 2018

I think the scenario you're describing is this:

  1. You have two sets of credentials for say, DockerHub, credsA and credsB
  2. You have three images with different access. imageA can be read only be credsA; imageB can be read only by credsB; imageC can be read by both.
  3. You set up two deployments, one using imageA and imageC and referring to credsA, and one using imageB and imageC and referring to credsB.

At this point, you see errors in the log, as fluxd tries to scan (say) imageA using credsB.

According to the algorithm, this is what should happen:

# for each workload:
#  merge all the secrets associated with it, as Kubernetes would do
#  enter the merged credentials against each image used by the workload
imageCreds = {}
workload = deploymentAC
mergedCreds = credsA
imageCreds[imageA] = credsA
imageCreds[imageC] = credsA

workload = deploymentBC
mergedCreds = credsB
imageCreds[imageB] = credsB
imageCreds[imageC] = credsB

When scanning imageA, it can only use credsA; and when scanning imageB, it can only use credsB; when scanning imageC, it can use credsA or credsB, depending on the order it looked at the workloads, but either is fine.

My conclusion is that the code doesn't implement the algorithm as given. Or I've misinterpreted your description of the test case.

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

Successfully merging a pull request may close this issue.

2 participants