Skip to content

Commit 5b4ec37

Browse files
rabirabi
committed
Support per-scope sigstore remap targets
Allow one ClusterImagePolicy per mirrored scope by carrying signedPrefix in each registry mapping while still rejecting conflicting per-scope matches. Change-Id: I1e0d4eb02be6912ab737550eb798f129ebac566e Signed-off-by: rabi <ramishra@redhat.com>
1 parent 51d623c commit 5b4ec37

3 files changed

Lines changed: 163 additions & 49 deletions

File tree

‎internal/dataplane/inventory.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,9 +171,6 @@ func GenerateNodeSetInventory(ctx context.Context, helper *helper.Helper,
171171
nodeSetGroup.Vars["edpm_container_signature_verification"] = true
172172
nodeSetGroup.Vars["edpm_container_signature_registry_mappings"] = sigstorePolicy.RegistryMappings
173173
nodeSetGroup.Vars["edpm_container_signature_cosign_key_data"] = sigstorePolicy.CosignKeyData
174-
if sigstorePolicy.SignedPrefix != "" {
175-
nodeSetGroup.Vars["edpm_container_signature_signed_prefix"] = sigstorePolicy.SignedPrefix
176-
}
177174
} else {
178175
helper.GetLogger().Info("No matching ClusterImagePolicy found; skipping sigstore verification")
179176
}

‎internal/dataplane/util/image_registry.go‎

Lines changed: 85 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -227,19 +227,17 @@ func getMachineConfig(ctx context.Context, helper *helper.Helper) (mc.MachineCon
227227
return masterMachineConfig, nil
228228
}
229229

230-
// RegistryMapping pairs a mirror registry with its upstream source from IDMS/ICSP.
230+
// RegistryMapping pairs a mirror with its upstream source.
231231
type RegistryMapping struct {
232-
Mirror string `json:"mirror"`
233-
Source string `json:"source"`
232+
Mirror string `json:"mirror"`
233+
Source string `json:"source"`
234+
SignedPrefix string `json:"signedPrefix,omitempty"`
234235
}
235236

236-
// SigstorePolicyInfo contains the EDPM-relevant parts of a ClusterImagePolicy.
237-
// A single RemapIdentity signedPrefix covers all mirrors under the same registry
238-
// root — the container runtime replaces only the prefix, preserving namespace paths.
237+
// SigstorePolicyInfo contains the EDPM sigstore settings.
239238
type SigstorePolicyInfo struct {
240239
RegistryMappings []RegistryMapping
241240
CosignKeyData string
242-
SignedPrefix string
243241
}
244242

245243
const (
@@ -319,6 +317,24 @@ func listClusterImagePolicies(
319317
return policyList, nil
320318
}
321319

320+
func collectMatchedMirrorScopes(policyScopes []string, mirrorScopes []string) []string {
321+
matchedMirrorScopes := map[string]struct{}{}
322+
for _, scope := range policyScopes {
323+
policyScope := normalizeImageScope(scope)
324+
if policyScope == "" {
325+
continue
326+
}
327+
328+
for _, mirrorScope := range mirrorScopes {
329+
if clusterImagePolicyScopeMatchesMirror(policyScope, mirrorScope) {
330+
matchedMirrorScopes[mirrorScope] = struct{}{}
331+
}
332+
}
333+
}
334+
335+
return sortedSetKeys(matchedMirrorScopes)
336+
}
337+
322338
// GetSigstoreImagePolicy checks if OCP has a ClusterImagePolicy configured
323339
// with sigstore signature verification for one of the mirror registries in use.
324340
// sourceByMirror maps each mirror scope to its upstream source registry (from IDMS/ICSP).
@@ -345,8 +361,25 @@ func GetSigstoreImagePolicy(ctx context.Context, helper *helper.Helper, mirrorSc
345361
return nil, nil
346362
}
347363

348-
var matches []string
349-
var match *SigstorePolicyInfo
364+
type policyMatch struct {
365+
name string
366+
keyData string
367+
signedPrefix string
368+
}
369+
370+
normalizedMirrors := make([]string, 0, len(mirrorScopes))
371+
sourceByNormalizedMirror := make(map[string]string, len(sourceByMirror))
372+
for _, mirrorScope := range mirrorScopes {
373+
normalizedMirror := normalizeImageScope(mirrorScope)
374+
if normalizedMirror == "" {
375+
continue
376+
}
377+
normalizedMirrors = append(normalizedMirrors, normalizedMirror)
378+
sourceByNormalizedMirror[normalizedMirror] = sourceByMirror[mirrorScope]
379+
}
380+
sort.Strings(normalizedMirrors)
381+
382+
matchByMirror := map[string]policyMatch{}
350383

351384
for _, policy := range policyList.Items {
352385
if policy.GetName() == "openshift" {
@@ -392,47 +425,57 @@ func GetSigstoreImagePolicy(ctx context.Context, helper *helper.Helper, mirrorSc
392425
}
393426
}
394427

395-
matchedMirrorScopes := map[string]struct{}{}
396-
for _, scope := range scopes {
397-
policyScope := normalizeImageScope(scope)
398-
if policyScope == "" {
399-
continue
400-
}
428+
matchedMirrors := collectMatchedMirrorScopes(scopes, normalizedMirrors)
429+
if len(matchedMirrors) == 0 {
430+
continue
431+
}
401432

402-
for _, mirrorScope := range mirrorScopes {
403-
if clusterImagePolicyScopeMatchesMirror(policyScope, mirrorScope) {
404-
matchedMirrorScopes[normalizeImageScope(mirrorScope)] = struct{}{}
405-
}
406-
}
433+
match := policyMatch{
434+
name: policy.GetName(),
435+
keyData: keyData,
436+
signedPrefix: signedPrefix,
407437
}
408-
if len(matchedMirrorScopes) == 0 {
409-
continue
438+
for _, mirror := range matchedMirrors {
439+
if existing, found := matchByMirror[mirror]; found {
440+
return nil, fmt.Errorf(
441+
"mirror scope %s matched multiple ClusterImagePolicies: %s, %s",
442+
mirror, existing.name, policy.GetName(),
443+
)
444+
}
445+
matchByMirror[mirror] = match
410446
}
447+
}
411448

412-
sortedMirrors := sortedSetKeys(matchedMirrorScopes)
449+
if len(matchByMirror) == 0 {
450+
return nil, nil
451+
}
413452

414-
mappings := make([]RegistryMapping, 0, len(sortedMirrors))
415-
for _, m := range sortedMirrors {
416-
mappings = append(mappings, RegistryMapping{
417-
Mirror: m,
418-
Source: sourceByMirror[m],
419-
})
420-
}
453+
sortedMirrors := make([]string, 0, len(matchByMirror))
454+
for mirror := range matchByMirror {
455+
sortedMirrors = append(sortedMirrors, mirror)
456+
}
457+
sort.Strings(sortedMirrors)
421458

422-
matches = append(matches, fmt.Sprintf("%s (%s)", policy.GetName(), strings.Join(sortedMirrors, ", ")))
423-
match = &SigstorePolicyInfo{
424-
RegistryMappings: mappings,
425-
CosignKeyData: keyData,
426-
SignedPrefix: signedPrefix,
427-
}
459+
firstMatch := matchByMirror[sortedMirrors[0]]
460+
match := &SigstorePolicyInfo{
461+
RegistryMappings: make([]RegistryMapping, 0, len(sortedMirrors)),
462+
CosignKeyData: firstMatch.keyData,
428463
}
429464

430-
if len(matches) > 1 {
431-
sort.Strings(matches)
432-
return nil, fmt.Errorf(
433-
"expected exactly one ClusterImagePolicy matching mirror registries, found %d: %s",
434-
len(matches), strings.Join(matches, ", "),
435-
)
465+
for _, mirror := range sortedMirrors {
466+
mirrorMatch := matchByMirror[mirror]
467+
if match.CosignKeyData != mirrorMatch.keyData {
468+
return nil, fmt.Errorf(
469+
"matched ClusterImagePolicies use different cosign key data and cannot be combined: %s, %s",
470+
firstMatch.name, mirrorMatch.name,
471+
)
472+
}
473+
474+
match.RegistryMappings = append(match.RegistryMappings, RegistryMapping{
475+
Mirror: mirror,
476+
Source: sourceByNormalizedMirror[mirror],
477+
SignedPrefix: mirrorMatch.signedPrefix,
478+
})
436479
}
437480

438481
return match, nil

‎internal/dataplane/util/image_registry_test.go‎

Lines changed: 78 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -778,10 +778,9 @@ func TestGetSigstoreImagePolicy_WithRemapIdentity(t *testing.T) {
778778
g.Expect(err).ToNot(HaveOccurred())
779779
g.Expect(result).ToNot(BeNil())
780780
g.Expect(result.RegistryMappings).To(Equal([]RegistryMapping{
781-
{Mirror: "local-registry.example.com:5000", Source: "registry.example.com/vendor"},
781+
{Mirror: "local-registry.example.com:5000", Source: "registry.example.com/vendor", SignedPrefix: "registry.example.com/vendor"},
782782
}))
783783
g.Expect(result.CosignKeyData).To(Equal(base64.StdEncoding.EncodeToString([]byte("test-public-key"))))
784-
g.Expect(result.SignedPrefix).To(Equal("registry.example.com/vendor"))
785784
}
786785

787786
func TestGetSigstoreImagePolicy(t *testing.T) {
@@ -806,7 +805,6 @@ func TestGetSigstoreImagePolicy(t *testing.T) {
806805
{Mirror: "local-registry.example.com:5000"},
807806
}))
808807
g.Expect(result.CosignKeyData).To(Equal(base64.StdEncoding.EncodeToString([]byte("test-public-key"))))
809-
g.Expect(result.SignedPrefix).To(BeEmpty())
810808
}
811809

812810
func TestGetSigstoreImagePolicy_ReturnsAllMatchingMirrorScopes(t *testing.T) {
@@ -886,7 +884,83 @@ func TestGetSigstoreImagePolicy_ReturnsErrorForAmbiguousPolicies(t *testing.T) {
886884

887885
result, err := GetSigstoreImagePolicy(ctx, h, []string{"mirror.example.com:5000/openstack-k8s-operators"}, nil)
888886
g.Expect(err).To(HaveOccurred())
889-
g.Expect(err.Error()).To(ContainSubstring("expected exactly one ClusterImagePolicy matching mirror registries"))
887+
g.Expect(err.Error()).To(ContainSubstring("mirror scope mirror.example.com:5000/openstack-k8s-operators matched multiple ClusterImagePolicies"))
888+
g.Expect(result).To(BeNil())
889+
}
890+
891+
func TestGetSigstoreImagePolicy_AllowsDifferentPoliciesPerMirrorScope(t *testing.T) {
892+
g := NewWithT(t)
893+
ctx := context.Background()
894+
895+
policy1 := newSigstorePolicy(
896+
"v1alpha1",
897+
"policy-rhoso",
898+
[]string{"mirror.example.com:5000/rhoso"},
899+
"shared-key",
900+
"RemapIdentity",
901+
"registry.redhat.io/rhoso",
902+
)
903+
policy2 := newSigstorePolicy(
904+
"v1alpha1",
905+
"policy-rhoso-operators",
906+
[]string{"mirror.example.com:5000/rhoso-operators"},
907+
"shared-key",
908+
"RemapIdentity",
909+
"registry.redhat.io/rhoso-operators",
910+
)
911+
912+
h := setupTestHelper(true, newClusterImagePolicyCRD("v1alpha1"), policy1, policy2)
913+
914+
sourceByMirror := map[string]string{
915+
"mirror.example.com:5000/rhoso": "registry.redhat.io/rhoso",
916+
"mirror.example.com:5000/rhoso-operators": "registry.redhat.io/rhoso-operators",
917+
}
918+
result, err := GetSigstoreImagePolicy(ctx, h, []string{
919+
"mirror.example.com:5000/rhoso",
920+
"mirror.example.com:5000/rhoso-operators",
921+
}, sourceByMirror)
922+
g.Expect(err).ToNot(HaveOccurred())
923+
g.Expect(result).ToNot(BeNil())
924+
g.Expect(result.RegistryMappings).To(Equal([]RegistryMapping{
925+
{Mirror: "mirror.example.com:5000/rhoso", Source: "registry.redhat.io/rhoso", SignedPrefix: "registry.redhat.io/rhoso"},
926+
{Mirror: "mirror.example.com:5000/rhoso-operators", Source: "registry.redhat.io/rhoso-operators", SignedPrefix: "registry.redhat.io/rhoso-operators"},
927+
}))
928+
g.Expect(result.CosignKeyData).To(Equal(base64.StdEncoding.EncodeToString([]byte("shared-key"))))
929+
}
930+
931+
func TestGetSigstoreImagePolicy_ReturnsErrorForDifferentKeysAcrossPolicies(t *testing.T) {
932+
g := NewWithT(t)
933+
ctx := context.Background()
934+
935+
policy1 := newSigstorePolicy(
936+
"v1alpha1",
937+
"policy-rhoso",
938+
[]string{"mirror.example.com:5000/rhoso"},
939+
"key-one",
940+
"RemapIdentity",
941+
"registry.redhat.io/rhoso",
942+
)
943+
policy2 := newSigstorePolicy(
944+
"v1alpha1",
945+
"policy-ubi9",
946+
[]string{"mirror.example.com:5000/ubi9"},
947+
"key-two",
948+
"RemapIdentity",
949+
"registry.redhat.io/ubi9",
950+
)
951+
952+
h := setupTestHelper(true, newClusterImagePolicyCRD("v1alpha1"), policy1, policy2)
953+
954+
sourceByMirror := map[string]string{
955+
"mirror.example.com:5000/rhoso": "registry.redhat.io/rhoso",
956+
"mirror.example.com:5000/ubi9": "registry.redhat.io/ubi9",
957+
}
958+
result, err := GetSigstoreImagePolicy(ctx, h, []string{
959+
"mirror.example.com:5000/rhoso",
960+
"mirror.example.com:5000/ubi9",
961+
}, sourceByMirror)
962+
g.Expect(err).To(HaveOccurred())
963+
g.Expect(err.Error()).To(ContainSubstring("different cosign key data"))
890964
g.Expect(result).To(BeNil())
891965
}
892966

0 commit comments

Comments
 (0)