mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 14:02:56 +09:00
fix(marketplace): preserve verify and reset the pin on repeat connect (API)
The connect API replaced the whole OCIRepository spec on a repeat connect, so a spec.verify signature policy added earlier was silently dropped. It now updates url, interval, ref and secretRef from the request while preserving an existing spec.verify: a repeat connect never silently downgrades trust. An explicit digest reference is pinned on the source directly; a tag-only connect is left for the operator to pin. Replacing the whole ref on a repeat connect drops the operator's prior digest pin so the newly requested tag re- resolves, and the pinned-digest annotation is cleared alongside the materialized-revision annotation so the operator re-pins and re-materializes. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
This commit is contained in:
2 files changed
+98
-7
No files matched your search
@@ -217,6 +217,15 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
|
||||
}
|
||||
}
|
||||
|
||||
// A connect carries a tag, a digest, or both; an explicit digest is pinned
|
||||
// here, while a tag-only source is pinned to a digest later by the operator.
|
||||
refSpec := map[string]any{}
|
||||
if target.Tag != "" {
|
||||
refSpec["tag"] = target.Tag
|
||||
}
|
||||
if target.Digest != "" {
|
||||
refSpec["digest"] = target.Digest
|
||||
}
|
||||
repo := &unstructured.Unstructured{Object: map[string]any{
|
||||
"apiVersion": "source.toolkit.fluxcd.io/v1",
|
||||
"kind": "OCIRepository",
|
||||
@@ -229,7 +238,7 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
|
||||
"spec": map[string]any{
|
||||
"url": target.URL,
|
||||
"interval": "5m0s",
|
||||
"ref": map[string]any{"tag": target.Tag},
|
||||
"ref": refSpec,
|
||||
},
|
||||
}}
|
||||
if in.Spec.SecretRef != "" {
|
||||
@@ -255,12 +264,22 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
|
||||
return nil, apierrors.NewConflict(r.gvr.GroupResource(), target.FluxSourceName,
|
||||
fmt.Errorf("a different repository (%s) is already connected as %q; disconnect it before connecting %s", curURL, target.FluxSourceName, target.URL))
|
||||
}
|
||||
// Update only the fields this API owns (spec, the tap label and name
|
||||
// annotation) on the FETCHED object, so the operator's finalizer survives.
|
||||
// Clear the materialized-revision annotation so a re-connect re-materializes
|
||||
// and recovers a registration Package (or PackageSource) removed out-of-band
|
||||
// since the last materialization, the same recovery `cozypkg tap` performs.
|
||||
cur.Object["spec"] = repo.Object["spec"]
|
||||
// Update the fields this API owns on the FETCHED object, so the operator's
|
||||
// finalizer survives. Replace url, interval, ref and secretRef from this
|
||||
// connect, but PRESERVE a spec.verify set earlier (by this API or by an
|
||||
// admin): a repeat connect must never silently drop a signature-verification
|
||||
// policy. Replacing the whole ref rather than merging it is deliberate: it
|
||||
// drops the operator's prior digest pin so the newly requested tag re-
|
||||
// resolves (the operator re-pins it). A connect that itself carries verify
|
||||
// overrides the preserved one.
|
||||
newSpec := map[string]any{}
|
||||
if existingVerify, ok, _ := unstructured.NestedMap(cur.Object, "spec", "verify"); ok {
|
||||
newSpec["verify"] = existingVerify
|
||||
}
|
||||
for k, v := range repo.Object["spec"].(map[string]any) {
|
||||
newSpec[k] = v
|
||||
}
|
||||
cur.Object["spec"] = newSpec
|
||||
labels := cur.GetLabels()
|
||||
if labels == nil {
|
||||
labels = map[string]string{}
|
||||
@@ -272,7 +291,12 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
|
||||
ann = map[string]string{}
|
||||
}
|
||||
ann[tapconst.NameAnnotation] = target.FluxSourceName
|
||||
// Clear the materialized-revision annotation so a re-connect re-materializes
|
||||
// (recovering a registration removed out-of-band, the same recovery `cozypkg
|
||||
// tap` performs), and the pinned-digest annotation so the operator re-pins
|
||||
// the newly requested tag rather than treating the dropped pin as a user digest.
|
||||
delete(ann, tapconst.MaterializedRevisionAnnotation)
|
||||
delete(ann, tapconst.PinnedDigestAnnotation)
|
||||
cur.SetAnnotations(ann)
|
||||
if _, err := src.Update(ctx, cur, metav1.UpdateOptions{FieldManager: "cozystack-api", DryRun: createDryRun(opts)}); err != nil {
|
||||
return nil, apierrors.NewInternalError(fmt.Errorf("update Flux source for tap %s: %w", target.FluxSourceName, err))
|
||||
|
||||
@@ -309,6 +309,73 @@ func TestCreateRepeatPreservesFinalizerAndForcesRematerialize(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestCreatePinsExplicitDigest(t *testing.T) {
|
||||
const digest = "sha256:1111111111111111111111111111111111111111111111111111111111111111"
|
||||
r := fakeREST()
|
||||
in := &corev1alpha1.Tap{Spec: corev1alpha1.TapSpec{URL: "oci://ghcr.io/foo/bar@" + digest}}
|
||||
if _, err := r.Create(context.Background(), in, nil, nil); err != nil {
|
||||
t.Fatalf("create: %v", err)
|
||||
}
|
||||
u, err := r.dyn.Resource(gvrOCIRepos).Namespace("cozy-system").Get(context.Background(), "tap-foo-bar", metav1.GetOptions{})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got, _, _ := unstructured.NestedString(u.Object, "spec", "ref", "digest"); got != digest {
|
||||
t.Errorf("explicit digest not pinned on the source: got %q", got)
|
||||
}
|
||||
// A digest-only connect must not also follow a tag.
|
||||
if tag, found, _ := unstructured.NestedString(u.Object, "spec", "ref", "tag"); found && tag != "" {
|
||||
t.Errorf("digest connect must not set a tag, got %q", tag)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCreateRepeatPreservesVerifyAndResetsPin(t *testing.T) {
|
||||
// A source connected earlier, pinned by the operator to a digest, with a
|
||||
// signature-verification policy added by an admin. A repeat connect at a new
|
||||
// tag must: preserve spec.verify (never silently downgrade trust), drop the
|
||||
// operator's digest pin so the new tag re-resolves, and clear the pin and
|
||||
// materialized-revision annotations so the operator re-pins and re-materializes.
|
||||
const oldDigest = "sha256:2222222222222222222222222222222222222222222222222222222222222222"
|
||||
existing := ociRepoObj("tap-foo-bar")
|
||||
_ = unstructured.SetNestedField(existing.Object, "oci://ghcr.io/foo/bar", "spec", "url")
|
||||
_ = unstructured.SetNestedField(existing.Object, oldDigest, "spec", "ref", "digest")
|
||||
_ = unstructured.SetNestedField(existing.Object, "v1", "spec", "ref", "tag")
|
||||
_ = unstructured.SetNestedMap(existing.Object, map[string]any{
|
||||
"provider": "cosign",
|
||||
"matchOIDCIdentity": []any{map[string]any{"issuer": "https://token.actions.githubusercontent.com", "subject": "https://github.com/org/repo/.github/workflows/release.yml@refs/tags/v1"}},
|
||||
}, "spec", "verify")
|
||||
existing.SetFinalizers([]string{"apps.cozystack.io/tap-materializer"})
|
||||
existing.SetAnnotations(map[string]string{
|
||||
"apps.cozystack.io/materialized-revision": "v1@" + oldDigest,
|
||||
"apps.cozystack.io/tap-pinned-digest": oldDigest,
|
||||
})
|
||||
r := fakeREST(existing)
|
||||
|
||||
in := &corev1alpha1.Tap{Spec: corev1alpha1.TapSpec{URL: "oci://ghcr.io/foo/bar:v2"}}
|
||||
if _, err := r.Create(context.Background(), in, nil, nil); err != nil {
|
||||
t.Fatalf("repeat create: %v", err)
|
||||
}
|
||||
u, err := r.dyn.Resource(gvrOCIRepos).Namespace("cozy-system").Get(context.Background(), "tap-foo-bar", metav1.GetOptions{})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if prov, _, _ := unstructured.NestedString(u.Object, "spec", "verify", "provider"); prov != "cosign" {
|
||||
t.Errorf("spec.verify was dropped on repeat connect: %+v", u.Object["spec"])
|
||||
}
|
||||
if tag, _, _ := unstructured.NestedString(u.Object, "spec", "ref", "tag"); tag != "v2" {
|
||||
t.Errorf("tag not updated to v2, got %q", tag)
|
||||
}
|
||||
if d, found, _ := unstructured.NestedString(u.Object, "spec", "ref", "digest"); found && d != "" {
|
||||
t.Errorf("operator digest pin must be dropped so the new tag re-resolves, got %q", d)
|
||||
}
|
||||
if _, ok := u.GetAnnotations()["apps.cozystack.io/tap-pinned-digest"]; ok {
|
||||
t.Error("pinned-digest annotation must be cleared on re-connect")
|
||||
}
|
||||
if _, ok := u.GetAnnotations()["apps.cozystack.io/materialized-revision"]; ok {
|
||||
t.Error("materialized-revision annotation must be cleared on re-connect")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCreateRefusesConflictingURL(t *testing.T) {
|
||||
// A source with the same derived name but a different registry host must not
|
||||
// be silently retargeted.
|
||||
|
||||
Reference in new issue
Block a user