Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ensure OCI artifacts are handled strictly by digest #1245

Open
errordeveloper opened this issue Sep 29, 2023 · 4 comments
Open

ensure OCI artifacts are handled strictly by digest #1245

errordeveloper opened this issue Sep 29, 2023 · 4 comments
Labels
area/oci OCI related issues and pull requests bug Something isn't working help wanted Extra attention is needed

Comments

@errordeveloper
Copy link
Contributor

Currently artifact revision (i.e. digest) is obtain here:

// Get the upstream revision from the artifact digest
revision, err := r.getRevision(url, opts.craneOpts)

It is also observed as a condition here:

message := fmt.Sprintf("new revision '%s' for '%s'", revision, url)
if obj.GetArtifact() != nil {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", message)
}
rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, "building artifact: %s", message)
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
ctrl.LoggerFrom(ctx).Error(err, "failed to patch")
return
}
}

However, verification and fetching is only done by URL, and it's possible there is an update in registry in between all of these calls:

err := r.verifySignature(ctx, obj, url, opts.verifyOpts...)

// Pull artifact from the remote container registry
img, err := crane.Pull(url, opts.craneOpts...)

There maybe other race coditions. It will be easy enough to address this and reinfoce use of the same digest for all of the registry API calls.

@errordeveloper errordeveloper changed the title ensure OCI artifact revisions are more thruthful. ensure OCI artifacts are handled strictly by digest Sep 29, 2023
@stefanprodan
Copy link
Member

stefanprodan commented Sep 29, 2023

We could make getRevision return also the digestURL and pass that to all sequential functions.

@stefanprodan stefanprodan added bug Something isn't working help wanted Extra attention is needed area/oci OCI related issues and pull requests labels Sep 29, 2023
@errordeveloper
Copy link
Contributor Author

#1244 will changed some of the underlying code, but this flow remains intact.

@errordeveloper
Copy link
Contributor Author

errordeveloper commented Sep 29, 2023

We could make getRevision return also the digestURL and pass that to all sequential functions.

Exactly! I would also want to check how artifacts are stored as well, as I'm not familiar with that part yet.

@stefanprodan
Copy link
Member

Flux has an internal artifact format common to all source types which is a gzip tarball with stripped file header info. The digest of a Flux artifact is advertised in .status.artifact.digest and all consumers (kustomize-controller, helm-controller, tf-controller, etc) verify the integrity of the artifact before the contents is extracted. The internal artifacts acquisition and verification is handled by https://github.com/fluxcd/pkg/blob/main/http/fetch/archive_fetcher.go.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
area/oci OCI related issues and pull requests bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants