Add platform filtering support to mapping.yml #167

kipz · 2024-09-17T14:16:09Z

This will help us roll out policy platform by platform, and control the amount of historical image signing we will need

e.g. only linux/amd64 images will be matched by the rule below:

version: v1
kind: policy-mapping
policies:
  - id: docker-official-images
    description: Docker Official Images
    files:
      - path: doi/policy.rego
rules:
  - pattern: "^public[.]ecr[.]aws/docker/library/(.*)$"
    platforms: ["linux/amd64"]
    rewrite: docker.io/library/$1

Also, rename config package to mapping, and move matching code over there from policy package

codecov · 2024-09-17T14:17:28Z

Codecov Report

Attention: Patch coverage is 90.80460% with 8 lines in your changes missing coverage. Please review.

Project coverage is 69.58%. Comparing base (05caa95) to head (0ca448c).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
mapping/match.go	90.47%	4 Missing ⚠️
mapping/mapping.go	84.61%	1 Missing and 1 partial ⚠️
verify.go	81.81%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #167      +/-   ##
==========================================
+ Coverage   69.46%   69.58%   +0.11%     
==========================================
  Files          44       44              
  Lines        2505     2528      +23     
==========================================
+ Hits         1740     1759      +19     
- Misses        475      477       +2     
- Partials      290      292       +2

Flag	Coverage Δ
unittests	`69.58% <90.80%> (+0.11%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

whalelines

A few questions

whalelines · 2024-09-17T20:20:45Z

mapping/mapping.go

@@ -1,4 +1,4 @@
-package config
+package mapping


That feels right

whalelines · 2024-09-17T20:24:32Z

mapping/testdata/mappings/doi-platform/mapping.yaml

+      - path: doi/policy.rego
+rules:
+  - pattern: "^docker[.]io/library/(.*)$"
+    platforms: ["linux/amd64"]


Should there be a test with two values?

well, it's an array, so I think the same paths will be exercised...happy to add one if you like?

whalelines · 2024-09-17T20:27:49Z

verify.go

@@ -55,7 +55,12 @@ func (verifier *tufVerifier) Verify(ctx context.Context, src *oci.ImageSpec) (re
 		return nil, fmt.Errorf("failed to resolve image name: %w", err)
 	}
 	policyResolver := policy.NewResolver(verifier.tufClient, verifier.opts)
-	resolvedPolicy, err := policyResolver.ResolvePolicy(ctx, imageName)
+
+	platform, err := detailsResolver.ImagePlatform(ctx)


Is platform used anywhere?

next line, yeah: https://github.com/docker/attest/pull/167/files#diff-32870c78c32d49e31ad4f22eaeac957ae6b988eea6fc937c9dba68035ac0d1b6R63

That's how the input image platform makes it down to the policy

@whalelines For better or worse, it would actually be a compiler error if it wasn't!

whalelines · 2024-09-17T20:30:32Z

mapping/match.go

+			case rule.PolicyID == "" && rule.Replacement == "":
+				return nil, fmt.Errorf("rule %s has neither policy-id nor rewrite", rule.Pattern)
+			case rule.PolicyID != "" && rule.Replacement != "":
+				return nil, fmt.Errorf("rule %s has both policy-id and rewrite", rule.Pattern)
+			case rule.PolicyID != "":
+				policy := mappings.Policies[rule.PolicyID]
+				if policy != nil {
+					return &PolicyMatch{
+						MatchType:   MatchTypePolicy,
+						Policy:      policy,
+						Rule:        rule,
+						MatchedName: imageName,
+					}, nil
+				}


This code seems very familiar. Does it exist somewhere else?

this was a move from the policy package.

mapping/match.go

README.md

mapping/match.go

mapping/match_test.go

jonnystoten · 2024-09-18T12:56:37Z

verify.go

@@ -55,7 +55,12 @@ func (verifier *tufVerifier) Verify(ctx context.Context, src *oci.ImageSpec) (re
 		return nil, fmt.Errorf("failed to resolve image name: %w", err)
 	}
 	policyResolver := policy.NewResolver(verifier.tufClient, verifier.opts)
-	resolvedPolicy, err := policyResolver.ResolvePolicy(ctx, imageName)
+
+	platform, err := detailsResolver.ImagePlatform(ctx)


@whalelines For better or worse, it would actually be a compiler error if it wasn't!

whalelines

LGTM

kipz requested a review from a team as a code owner September 17, 2024 14:16

github-actions bot added the chore label Sep 17, 2024

kipz enabled auto-merge (squash) September 17, 2024 14:16

whalelines reviewed Sep 17, 2024

View reviewed changes

kipz added 4 commits September 18, 2024 13:55

chore!: rename package config -> mapping

189a751

feat: add platform filtering support to mapping.yml

e498ffd

fix: readme text. uncomment tests

febeed4

add more platform filter tests

cf0970f

kipz force-pushed the kipz_platform branch from 7255290 to cf0970f Compare September 18, 2024 12:56

kipz requested a review from whalelines September 18, 2024 12:57

jonnystoten reviewed Sep 18, 2024

View reviewed changes

fix nits

1f495e3

kipz disabled auto-merge September 18, 2024 13:39

refactor: move platfor match to fn

0ca448c

jonnystoten approved these changes Sep 18, 2024

View reviewed changes

whalelines approved these changes Sep 18, 2024

View reviewed changes

kipz merged commit 4a70e5a into main Sep 18, 2024
9 checks passed

kipz deleted the kipz_platform branch September 18, 2024 20:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add platform filtering support to mapping.yml #167

Add platform filtering support to mapping.yml #167

kipz commented Sep 17, 2024 •

edited

Loading

codecov bot commented Sep 17, 2024 •

edited

Loading

whalelines left a comment

whalelines Sep 17, 2024

whalelines Sep 17, 2024

kipz Sep 18, 2024

kipz Sep 18, 2024

whalelines Sep 17, 2024

kipz Sep 18, 2024

kipz Sep 18, 2024

jonnystoten Sep 18, 2024

whalelines Sep 17, 2024

kipz Sep 18, 2024

jonnystoten Sep 18, 2024

whalelines left a comment

Add platform filtering support to mapping.yml #167

Add platform filtering support to mapping.yml #167

Conversation

kipz commented Sep 17, 2024 • edited Loading

codecov bot commented Sep 17, 2024 • edited Loading

Codecov Report

whalelines left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whalelines left a comment

Choose a reason for hiding this comment

kipz commented Sep 17, 2024 •

edited

Loading

codecov bot commented Sep 17, 2024 •

edited

Loading