-
Notifications
You must be signed in to change notification settings - Fork 3
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
Implement automatic distribution updates from OCI artifacts #33
Conversation
Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
33373a5
to
09e1350
Compare
curl -sLO https://github.com/controlplaneio-fluxcd/distribution/archive/refs/heads/main.tar.gz | ||
tar xzf main.tar.gz -C "${DEST_DIR}" | ||
|
||
mkdir -p "${IMG_DIR}" |
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.
maybe should add a check here?
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.
-p
will not create the dir if it exists.
// e.g. 'oci://ghcr.io/controlplaneio-fluxcd/flux-operator-manifests:latest'. | ||
// +kubebuilder:validation:Pattern="^oci://.*$" | ||
// +optional | ||
Artifact string `json:"artifact,omitempty"` |
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.
can we name it maybe artifactURL
?
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.
I did consider this but given that we use registry
and not registryURL
, for consistency I chose to not have the URL suffix.
e134caf
to
a2fd0f2
Compare
Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
a2fd0f2
to
98ba38b
Compare
Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
ctxPull, cancel := context.WithTimeout(context.Background(), 30*time.Second) | ||
defer cancel() | ||
|
||
artifactDigest, err := builder.PullArtifact(ctxPull, artifactURL, tmpDir) |
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.
If I got it correctly, at every reconciliation we fetch the manifests and don't store it locally. What do you think about storing the artifact
and its manifest
digest, at each reconciliation, we would first fetch the manifest
and compare the digest with local version before attempting to fetch the blob
?
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.
Yes we could have a similar storage & auth implementation as SC in a followup PR. I'm also considering using the layer caching I implemented in Timoni which is more close to how OverlayFS works in OCI registries.
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.
LGTM!
This PR adds a new optional filed named
.spec.distribution.artifact
to the FluxInstance API. When specified, the operator will pull the artifact from the registry on a regular interval to determine the latest Flux version available including CVE patches and hotfixes.Using this feature, CNCF Flux users can keep Flux up-to-date without having to update the operator every time there is a new Flux version available.
For enterprise customers, this feature allows for automated updates of CVE patches to fixed versions and semver ranges without having to update the operator.