Skip to content

Commit 0fd9703

Browse files
IvanHuntersIvanHunters
committed
fix(marketplace): preserve verify and pull secret on CLI re-tap
A re-tap applies the OCIRepository under field owner cozypkg via server-side apply; a field this owner set earlier and now omits is removed. Re-tapping to move the pin without repeating --certificate-identity/--certificate-oidc-issuer therefore silently dropped the signature-verification policy, and re-tapping a private repo without --secret dropped the pull credential. The CLI now carries an existing source's spec.verify and secretRef onto the applied object when the invocation does not set them, mirroring the API connect path, so moving the pin never silently downgrades trust or breaks a private pull. Assisted-by: LLM Signed-off-by: IvanHunters <[email protected]>
1 parent 5747f94 commit 0fd9703

2 files changed

Lines changed: 56 additions & 0 deletions

File tree

‎cmd/cozypkg/cmd/tap.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,24 @@ func buildTapOCIRepository(name string, r ociRef, secret string, verify *sourcev
180180
return obj
181181
}
182182

183+
// carryoverSourceFields preserves an existing tap source's verification policy
184+
// and pull credential when this invocation did not set them from flags, so a
185+
// re-tap (e.g. to move the pin to a new tag) never silently drops signature
186+
// verification or the pull secret. A server-side apply that omits a field this
187+
// field owner previously set would otherwise remove it, which the API connect
188+
// path (rest.go) deliberately avoids; this keeps the CLI symmetric with it.
189+
func carryoverSourceFields(obj, existing *sourcev1.OCIRepository, flagVerify *sourcev1.OCIRepositoryVerification, flagSecret string) {
190+
if existing == nil {
191+
return
192+
}
193+
if flagVerify == nil && existing.Spec.Verify != nil {
194+
obj.Spec.Verify = existing.Spec.Verify
195+
}
196+
if flagSecret == "" && existing.Spec.SecretRef != nil {
197+
obj.Spec.SecretRef = existing.Spec.SecretRef
198+
}
199+
}
200+
183201
// tapVerification builds a keyless cosign verification policy from the connect
184202
// flags, or nil when neither is set. Both the certificate identity (subject)
185203
// and the OIDC issuer are required together: a policy that verified a signature
@@ -396,6 +414,9 @@ runs its charts in your management cluster: tap only sources you trust.`,
396414
if existing.Spec.URL != "" && existing.Spec.URL != ref.URL {
397415
return fmt.Errorf("a different repository (%s) is already tapped as %q; run 'cozypkg untap %s' before tapping %s", existing.Spec.URL, srcName, names[0], ref.URL)
398416
}
417+
// Same repository, re-tapped: carry over a verify policy or pull secret
418+
// this invocation did not set, so moving the pin never silently drops them.
419+
carryoverSourceFields(ociRepo, existing, verify, tapCmdFlags.secret)
399420
}
400421

401422
// A tapped repository keeps its own declared name. Refuse the whole tap

‎cmd/cozypkg/cmd/tap_test.go‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"io"
2323
"testing"
2424

25+
fluxmeta "github.com/fluxcd/pkg/apis/meta"
2526
sourcev1 "github.com/fluxcd/source-controller/api/v1"
2627
apierrors "k8s.io/apimachinery/pkg/api/errors"
2728
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -207,6 +208,40 @@ func TestTapVerification(t *testing.T) {
207208
}
208209
}
209210

211+
func TestCarryoverSourceFields(t *testing.T) {
212+
verifyPol := &sourcev1.OCIRepositoryVerification{Provider: "cosign", MatchOIDCIdentity: []sourcev1.OIDCIdentityMatch{{Issuer: "iss", Subject: "subj"}}}
213+
existing := &sourcev1.OCIRepository{Spec: sourcev1.OCIRepositorySpec{
214+
Verify: verifyPol,
215+
SecretRef: &fluxmeta.LocalObjectReference{Name: "pull-creds"},
216+
}}
217+
218+
// Re-tap without flags: an existing verify policy and pull secret are carried
219+
// onto the object to apply, so a server-side apply that omits them does not
220+
// remove them (SSA prunes a previously-owned field left out of the apply).
221+
obj := &sourcev1.OCIRepository{}
222+
carryoverSourceFields(obj, existing, nil, "")
223+
if obj.Spec.Verify == nil {
224+
t.Error("existing verify policy must be preserved when no --certificate-* flags are given")
225+
}
226+
if obj.Spec.SecretRef == nil || obj.Spec.SecretRef.Name != "pull-creds" {
227+
t.Error("existing pull secret must be preserved when no --secret is given")
228+
}
229+
230+
// Flags present: the invocation's own values win, nothing is carried over.
231+
flagVerify := &sourcev1.OCIRepositoryVerification{Provider: "cosign"}
232+
obj2 := &sourcev1.OCIRepository{Spec: sourcev1.OCIRepositorySpec{Verify: flagVerify, SecretRef: &fluxmeta.LocalObjectReference{Name: "new-creds"}}}
233+
carryoverSourceFields(obj2, existing, flagVerify, "new-creds")
234+
if obj2.Spec.Verify != flagVerify {
235+
t.Error("a verify policy from flags must not be overwritten by the existing one")
236+
}
237+
if obj2.Spec.SecretRef.Name != "new-creds" {
238+
t.Error("a secret from --secret must not be overwritten by the existing one")
239+
}
240+
241+
// No existing source: a no-op, no panic.
242+
carryoverSourceFields(&sourcev1.OCIRepository{}, nil, nil, "")
243+
}
244+
210245
func TestBuildTapOCIRepositoryWithVerify(t *testing.T) {
211246
r := ociRef{URL: "oci://ghcr.io/foo/bar", Repo: "bar", Tag: "v1"}
212247
v := &sourcev1.OCIRepositoryVerification{

0 commit comments

Comments
 (0)