-
Notifications
You must be signed in to change notification settings - Fork 192
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
Add support for using an OCI image as source #450
Conversation
Thank you very much for this initial contribution @nebhale. We are at present in the process of bringing some standardization to the set of controllers, including better tests, and you may want to have a look at the |
Hi @nebhale could you please take a look at my proposal for the API here: fluxcd/flux2#1705 (comment) Here is an example of how I see the API: apiVersion: source.toolkit.fluxcd.io/v1beta1
kind: OCIRepository
metadata:
name: podinfo-release-candidates
namespace: flux-system
spec:
interval: 10m
image: ghcr.io/stefanprodan/podinfo-deploy
secretRef:
name: regcred
filterTags:
pattern: '.*-rc.*'
policy:
semver:
range: '^1.x-0' |
@stefanprodan I'll update to match your proposal. |
I would rather see this as a self-contained controller. Sources should really be a point of extension (that is, people should be able to provide their own kinds of source as self-contained controllers, from which users can pick and choose). At present this isn't possible because the source types are hard-coded into the controllers that use sources. @nebhale Would you be willing to collaborate on a design for making sources extensible? I know @hiddeco has already thought a lot about it. My other concern is that the API here ignores the existing API for specifying image repositories and selecting amongst the images therein: https://fluxcd.io/docs/components/image/imagepolicies/. The types there are for selecting container images for automating updates to workloads, so a different purpose (though it's conceivable you could automation updates to config images ...). Nonetheless a user might reasonably ask "Why can't I specify an image source using a semver range?" or just "Why are there two different schemes for referring to images?". |
@stefanprodan as I was implementing I was thinking about where apiVersion: source.toolkit.fluxcd.io/v1beta1
kind: ArtifactRepository
metadata:
name: podinfo-release-candidates
namespace: flux-system
spec:
interval: 10m
image: ghcr.io/stefanprodan/podinfo-deploy
secretRef:
name: regcred
digest: sha256:d8ba3e9dc883b854a30c40ccdf6d6653b26868c7b77351d4c91eaffaf662611e apiVersion: source.toolkit.fluxcd.io/v1beta1
kind: ArtifactRepository
metadata:
name: podinfo-release-candidates
namespace: flux-system
spec:
interval: 10m
image: ghcr.io/stefanprodan/podinfo-deploy
secretRef:
name: regcred
tag: '1.0.0' apiVersion: source.toolkit.fluxcd.io/v1beta1
kind: ArtifactRepository
metadata:
name: podinfo-release-candidates
namespace: flux-system
spec:
interval: 10m
image: ghcr.io/stefanprodan/podinfo-deploy
secretRef:
name: regcred
filterTags:
pattern: '^main-[a-fA-F0-9]+-(?P<ts>.*)'
extract: '$ts'
policy:
numerical:
order: asc apiVersion: source.toolkit.fluxcd.io/v1beta1
kind: ArtifactRepository
metadata:
name: podinfo-release-candidates
namespace: flux-system
spec:
interval: 10m
image: ghcr.io/stefanprodan/podinfo-deploy
secretRef:
name: regcred
filterTags:
pattern: '.*-rc.*'
policy:
semver:
range: '^1.x-0' Also, what do you think about |
@squaremo I'd love to start engaging on the idea of source extensibility. We've got some experience in this subject and I think we can bring some alternative view points to the table. |
Either a hackmd.io document, or Google document would work for me (I probably need to create the outline myself, which means I also need some time). In short @nebhale: What I envision is the Consumers would only be aware of the This would allow for "wilcard" Is this inline with your experience on the subject, or do you have an alternative model in mind that may be a better fit? |
@hiddeco I don't think I quite grasp what you're driving at with your description. Do you have an example of what concrete resources you'd expect the user to create and then to have resulted when reconciliation is complete? I guess I just don't quite grasp the relationship between "Domain specific 'producers'" and the |
Domain specific 'producers' are things like At present the A user would still just create a An additional thing this would allow is a historical list of artifacts for an object (because there now is a one-to-many possibility, opposed to a one-to-one). Which could be an enabler for more advanced use-cases (rollbacks, diffs, ..?). |
I don't think that we currently have the issue that this is trying to address because we treat the the resource as a duck-type (we call it "Latest Source" but now that I've been in the Flux code, "Artifact" is the accurate name). When it comes time to consume any of the resource types currently exposed by the controller (or the ones that we've written directly), we don't actually deserialize into their concrete Go structs, but rather do a partial deserialization of the resource into our "Duck struct". The pattern is analogous to Go's consumer-side interfaces and doesn't require us to be aware of all possible types. For our usage, we prefer both creation of the resource and the return of |
The historical list of artifacts angle is a good one and probably quite useful to add to the domain resources. I think there are some positive analogs with kpack's Addition of |
@nebhale There are two related issues (but you can have one without the other):
You can avoid the first in your own consumer controller because you're in charge of the custom type and you can just use a reference without a constraint on the |
@squaremo A good example of this is in our implementation of the Service Binding for Kubernetes specification. In that, there is a I believe (and to be clear I've not looked deeply into Flux around this point) this same effect can be achieved for the two points that you've raised. In both cases, the controllers that consume a "Latest Artifact" conformant resource needn't know about any resource type whatsoever. Instead, they just use a struct LatestArtifact {
Status struct {
Artifact Artifact `json:"artifact"`
} `json:"status"`
} Since this is the contract of the thing you're trying to consume, you can safely ignore every other part of the resource payload and never know about any of it. |
This is the tricky bit: controller-runtime is very keen that you create watches up-front, by type. I'm sure it's possible to create and discard watches dynamically (it's all the same client-go machinery underneath), you're just on your own somewhat. My concern is about having extensibility; modulo complexity, I don't mind too much whether that's accomplished with exact types (via |
If users can install Flux source-extensions with their own unique GVK's that implement a Duck Type:
Important to think about whether source-extensions will use their own apiGroup, because that does add another string for a user to lookup/specify/ensure-is-correct which can affect usability.
|
It's great to be thinking forward towards extensibility. I imagine this will be used more often than
|
@squaremo I've often hoped https://fluxcd.io/docs/components/source/gitrepositories/ |
I can only speak for myself but I think we should ship |
I propose we include OCI in the Here is my proposal: apiVersion: source.toolkit.fluxcd.io/v1beta1
kind: OCIRepository
metadata:
name: podinfo-deploy
namespace: flux-system
spec:
interval: 10m
url: ghcr.io/stefanprodan/podinfo-deploy
ref:
# one of
tag: "latest"
digest: "sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2"
semver: "1.x"
auth:
# one of
secretRef:
name: regcred
serviceAccountName: reg
verify:
secretRef:
name: sigstore-keys The |
Revisiting my initial comments:
I would like to see the source protocol opened up to extension, but it doesn't have to hold this PR hostage. I do buy the argument made by Leigh that if you support buckets as a "core" type, you should probably support OCI artifacts. Let's move the architectural discussion to another venue -- in fact, I would like to see a more formal (RFC?) process for big design changes, Leigh's design for using impersonation, and opening up the source protocol, are both good candidates for RFCs.
I have reconciled myself to that ship having sailed for the v1 API 😭 Changing this is going to be a longer process, and certainly making the APIs consistent will have to wait for a v2(alpha1). Of course, once the source protocol is opened up, there's nothing stopping a third party from making their own controllers and APIs. In sum: I'm persuaded that getting this new kind of source into users' hands takes priority over redesigning APIs. Re the (second) schema suggestion from Stefan: this looks right to me. Putting your configs in OCI artifacts is going to be different to using git (because you'll probably have to use some CI to build them, for one thing). Nonetheless, following a tag (yuck, but people do it), or using a specific digest, or following a semver range is a decent set of alternatives to start with. I suspect people will think of config artifacts in much the same way as they think of container images, so I'd expect users to ask for the same flexibility with filtering and ordering that I don't think it's necessary to implement verification (or to have it in the API) before making the new kind of source available to people, if that's going to hold things up. The arguments for deferring filters, not reworking the API, etc. etc., can also be directed at verification. (Ultimately, the release schedule is a matter for the source-controller maintainers, though.) |
@stefanprodan, @squaremo, and @hiddeco thanks for the lively discussion and I'm looking forward to being involved as Flux evolves. I also deeply appreciate the pragmatic approach to getting the OCI Image functionality in place and then iterating towards a more generalized design across the whole system. Please look out for a concrete PR shortly with proposed changes and testing included. |
Late to the party, but thanks for the valuable insights @nebhale, this has been really fruitful to get more angles of what extensibility would look like, and how it would serve different usages so that we can ensure we evolve things into something that suits (almost) all. I am looking forward to the concrete PR, and (hopefully) further collaborations 🌻💯 |
I have prepared a repo for e2e tests and demos at The image contains the following manifests (which are taken from here): $ crane export ghcr.io/stefanprodan/podinfo-deploy:latest -| tar -tvf -
-rw-r--r-- 0 0 0 1713 Oct 21 11:44 deployment.yaml
-rw-r--r-- 0 0 0 419 Oct 21 11:44 hpa.yaml
-rw-r--r-- 0 0 0 126 Oct 21 11:44 kustomization.yaml
-rw-r--r-- 0 0 0 271 Oct 21 11:44 service.yaml
$ crane export ghcr.io/stefanprodan/podinfo-deploy:6.0.1 -| tar -Oxf - deployment.yaml | grep ghcr
image: ghcr.io/stefanprodan/podinfo:6.0.1
$ crane export ghcr.io/stefanprodan/podinfo-deploy:6.0.0 -| tar -Oxf - deployment.yaml | grep ghcr
image: ghcr.io/stefanprodan/podinfo:6.0.0 The images are signed with cosign: $ cat cosign.pub
-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEST+BqQ1XZhhVYx0YWQjdUJYIG5Lt
iz2+UxRIqmKBqNmce2T+l45qyqOs99qfD7gLNGmkVZ4vtJ9bM7FxChFczg==
-----END PUBLIC KEY-----
$ cosign verify --key cosign.pub ghcr.io/stefanprodan/podinfo-deploy
Verification for ghcr.io/stefanprodan/podinfo-deploy --
The following checks were performed on each of these signatures:
- The cosign claims were validated
- The signatures were verified against the specified public key
- Any certificates were verified against the Fulcio roots.
{"critical":{"identity":{"docker-reference":"ghcr.io/stefanprodan/podinfo-deploy"},"image":{"docker-manifest-digest":"sha256:2c70b816cf4213db92d1d95206aea5b79fa7d59d56fd7a4186c5d9b5b4c3f120"},"type":"cosign container image signature"},"optional":null} Using the above image we could swap the podinfo apiVersion: source.toolkit.fluxcd.io/v1beta1
kind: OCIRepository
metadata:
name: podinfo
namespace: flux-system
spec:
interval: 10m
url: ghcr.io/stefanprodan/podinfo-deploy
ref:
tag: "latest"
---
apiVersion: kustomize.toolkit.fluxcd.io/v1beta2
kind: Kustomization
metadata:
name: podinfo
namespace: flux-system
spec:
interval: 5m0s
path: "./"
prune: true
sourceRef:
kind: OCIRepository
name: podinfo
targetNamespace: default Currently I'm using crane to build the OCI atifact but ideally this will be incorporated into flux CLI e.g.
|
This change defines an OCIRepository CRD that allows a user to specify a given image to use as a source. The contents of the image (including images with multiple layers) are converted into a TAR and exposed to consumers following the same conventions as the other source types. apiVersion: source.toolkit.fluxcd.io/v1beta1 kind: OCIRepository metadata: name: podinfo-deploy namespace: flux-system spec: interval: 10m url: ghcr.io/stefanprodan/podinfo-deploy ref: # one of tag: "latest" digest: "sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2" semver: "1.x" auth: # one of secretRef: name: regcred serviceAccountName: reg In addition to the url, interval, timeout, ignore, and suspend keys (all of which behave consistently with the existing source types) this CRD also defines authentication via both an image pull Secret and a service serviceAccountName which provide ways to contribute registry connection credentials for the specified image. This change also adds a new way to write to the storage archive by streaming data from an incoming TAR without writing it to the filesystem. A couple of code and test functions were extracted to reuse common functionality for both archive strategies. Signed-off-by: Ben Hale <[email protected]>
} | ||
|
||
sort.Sort(sort.Reverse(semver.Collection(candidates))) | ||
return name.ParseReference(fmt.Sprintf("%s:%s", url, candidates[0])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The semver.NewVersion
strips the v
prefix, so we can't use it to build the URL, please use candidates[0].Original()
.
Will update this PR. RFC fluxcd/flux2#2601 pending |
Superseded by #788 |
This change defines an
OCIRepository
CRD that allows a user to specify a given image to use as a source. The contents of the image (including images with multiple layers) are converted into a TAR and exposed toconsumers following the same conventions as the other source types.
In addition to the
url
,interval
,timeout
,ignore
, andsuspend
keys (all of which behave consistently with the existing source types) this CRD also defines authentication via both an image pullSecret
and a serviceserviceAccountName
which provide ways to contribute registry connection credentials for the specified image.This change also adds a new way to write to the storage archive by streaming data from an incoming TAR without writing it to the filesystem. A couple of code and test functions were extracted to reuse common functionality for both archive strategies.
Formerly:
This change defines an
OCIImage
CRD that allows a user to specify a given image to use as source. The contents of the image (including images with multiple layers) are converted into a TAR and exposed to consumers following the same conventions as the other source types.In addition to the
image
,interval
,timeout
,ignore
, andsuspend
keys (all of which behave consistently with the existing source types) this CRD also defines bothimagePullSecrets
andserviceAccountName
keys which provide ways to contribute registry connection credentials for the specified image.This change also adds a new way to write to the storage archive by streaming data from an incoming TAR without writing it to the filesystem. A couple of code and test functions were extracted to reuse common functionality for both archive strategies.
Note: The controller does not yet have any tests although I do plan to contribute them. I took a look at the
gitrepository_controller_test.go
. noticed that it made reference to a package in another repo and decided to ask for your advice on testing, first. Should I make a coordinated separate contribution there, add an internal package here, or something else?