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

feat(digest) Parse images digest #2162

Closed
wants to merge 3 commits into from
Closed

Conversation

carlosjgp
Copy link

In some occasions the same tag can be assigned to different images.
Like for example the official Python images tagged with 3.7 or other
projects which may not have a labeling strategy which only uses a label
like latest or master. Using digests instead of labels ensures that
you don't receive unexpected upgrades

More info about tags and digests can be found
here
Fixes #885

In some occasions the same tag can be assigned to different images.
Like for example the official Python images tagged with `3.7` or other
projects which may not have a labeling strategy which only uses a label
like `latest` or `master`. Using digests instead of labels ensures that
you don't receive unexpected upgrades

More info about tags and digests can be found
[here](https://success.docker.com/article/images-tagging-vs-digests)

Fixes fluxcd#885
@squaremo
Copy link
Member

Great, thank you. Can you say a bit about how this change interacts with image updates? Or is it more or less invisible, except in that it avoids errors when parsing image refs that include digests.

@carlosjgp
Copy link
Author

@squaremo I haven't checked the image updated but, in my opinion, when someone uses a digest instead of a label or in conjunction with a label the intention is to not receive unexpected updates.

I'll create some test to ensure that what I just said is true.

@carlosjgp
Copy link
Author

@squaremo It took me quite a lot to figure out how to test the automated updates but I think I got it.

I've created a couple of tests to ensure that updates ignore Images with digests since they are supposed to be used when you don't want to receive updates.

@hiddeco
Copy link
Member

hiddeco commented Aug 1, 2019

@carlosjgp just wanted to let you know that we haven't forgotten about this PR and that I will likely be reviewing (and testing) this next Monday.

@hiddeco hiddeco self-assigned this Aug 1, 2019
@hiddeco
Copy link
Member

hiddeco commented Aug 5, 2019

@carlosjgp I gave it a test drive and noticed pretty soon that images from workloads in Kubernetes with just an image digest aren't parsed correctly.

Workloads used for testing:

---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: nginx
  labels:
    app: nginx
spec:
  replicas: 1
  selector:
    matchLabels:
      app: nginx
  template:
    metadata:
      labels:
        app: nginx
    spec:
      containers:
      - name: nginx
        image: nginx:1.7.9
        ports:
        - containerPort: 80
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: nginx-digest
  labels:
    app: nginx-digest
spec:
  replicas: 1
  selector:
    matchLabels:
      app: nginx-digest
  template:
    metadata:
      labels:
        app: nginx-digest
    spec:
      containers:
      - name: nginx
        image: nginx@sha256:e3456c851a152494c3e4ff5fcc26f240206abac0c9d794affb40e0714846c451
        ports:
        - containerPort: 80

Take for example fluxctl <list-workload|list-images>:

❯ fluxctl list-workloads
WORKLOAD                               CONTAINER           IMAGE                                                                                                                                                  RELEASE  POLICY
default:deployment/flux                flux                fluxcd/flux:latest                                                                                                                                     ready    
default:deployment/flux-helm-operator  flux-helm-operator  fluxcd/helm-operator:latest                                                                                                                            ready    
default:deployment/memcached           memcached           memcached:1.5.15                                                                                                                                       ready    
default:deployment/nginx               nginx               nginx:1.7.9                                                                                                                                            ready    
default:deployment/nginx-digest        nginx               nginx@sha256:e3456c851a152494c3e4ff5fcc26f240206abac0c9d794affb40e0714846c451@sha256:e3456c851a152494c3e4ff5fcc26f240206abac0c9d794affb40e0714846c451  ready    
❯ fluxctl list-images -w default:deployment/nginx-digest
WORKLOAD                         CONTAINER  IMAGE                                                                                                                                                  CREATED
default:deployment/nginx-digest  nginx      nginx@sha256:e3456c851a152494c3e4ff5fcc26f240206abac0c9d794affb40e0714846c451@sha256:e3456c851a152494c3e4ff5fcc26f240206abac0c9d794affb40e0714846c451  image data not available
                                            '-> (untagged)                                                                                                                                         ?

and from the logs:

ts=2019-08-05T12:31:41.406348903Z caller=warming.go:180 component=warmer canonical_name=index.docker.io/library/nginx@sha256 auth={map[]} err="requesting tags: mux: variable \"library/nginx@sha256\" doesn't match, expected \"^(?:(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])(?:(?:\\\\.(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]))+)?(?::[0-9]+)?/)?[a-z0-9]+(?:(?:(?:[._]|__|[-]*)[a-z0-9]+)+)?(?:(?:/[a-z0-9]+(?:(?:(?:[._]|__|[-]*)[a-z0-9]+)+)?)+)?$\""
ts=2019-08-05T12:32:38.562774778Z caller=warming.go:180 component=warmer canonical_name=index.docker.io/library/nginx@sha256 auth={map[]} err="requesting tags: mux: variable \"library/nginx@sha256\" doesn't match, expected \"^(?:(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])(?:(?:\\\\.(?:[a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9]))+)?(?::[0-9]+)?/)?[a-z0-9]+(?:(?:(?:[._]|__|[-]*)[a-z0-9]+)+)?(?:(?:/[a-z0-9]+(?:(?:(?:[._]|__|[-]*)[a-z0-9]+)+)?)+)?$\""

@carlosjgp
Copy link
Author

Thanks @hiddeco I'll create more tests to cover these scenarios and amend the code accordingly

@fabstu
Copy link

fabstu commented Jun 2, 2020

@carlosjgp did this die due to lack of interest or due to other problems? What workaround did you use?

@carlosjgp
Copy link
Author

Hi @fabstu ,
I just stopped using flux in favour of ArgoCD. And the fact that this changes did pass the automated tests but not some manual tests it made the work much harder and the feedback not as fast I would like.

I like contributing to open source projects but this was not an ideal experience

Sorry I didn't got this finished, please feel free to fork my branch and finish the work

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

Successfully merging this pull request may close these issues.

Handle image names with an image digest instead of a tag
4 participants