From 1f806f33a8f4156f9aed08126de2c33ce6a7bc30 Mon Sep 17 00:00:00 2001 From: James Carnegie Date: Tue, 3 Sep 2024 12:21:24 +0100 Subject: [PATCH] feat: validate mapping files on load (#147) --- config/config.go | 69 +++++++++++++++++++++++++++++------- config/config_test.go | 82 +++++++++++++++++++++++++++++++++++++++++++ policy/match_test.go | 19 +++------- 3 files changed, 143 insertions(+), 27 deletions(-) create mode 100644 config/config_test.go diff --git a/config/config.go b/config/config.go index 6eea642..6a626b3 100644 --- a/config/config.go +++ b/config/config.go @@ -1,6 +1,7 @@ package config import ( + "errors" "fmt" "os" "path/filepath" @@ -14,21 +15,69 @@ const ( MappingFilename = "mapping.yaml" ) +func validateMappingsFile(mappings *policyMappingsFile) error { + var validationErrors []error + if mappings.Kind != "policy-mapping" { + validationErrors = append(validationErrors, fmt.Errorf("file is not of kind policy-mapping: %s", mappings.Kind)) + } + if mappings.Version != "v1" { + validationErrors = append(validationErrors, fmt.Errorf("unsupported policy mapping file version: %s", mappings.Version)) + } + for _, rule := range mappings.Rules { + if rule.Pattern == "" { + validationErrors = append(validationErrors, fmt.Errorf("rule missing pattern: %s", rule)) + } + if rule.PolicyID == "" && rule.Replacement == "" { + validationErrors = append(validationErrors, fmt.Errorf("rule must have policy-id or replacement: %s", rule)) + } + if rule.PolicyID != "" && rule.Replacement != "" { + validationErrors = append(validationErrors, fmt.Errorf("rule cannot have both policy-id and replacement: %s", rule)) + } + } + for _, policy := range mappings.Policies { + if policy.ID == "" { + validationErrors = append(validationErrors, fmt.Errorf("policy missing id: %s", policy.ID)) + } + if len(policy.Files) == 0 { + validationErrors = append(validationErrors, fmt.Errorf("policy missing files: %v", policy)) + } + for _, file := range policy.Files { + if file.Path == "" { + validationErrors = append(validationErrors, fmt.Errorf("file missing path: %s", file)) + } + } + } + + if len(validationErrors) > 0 { + return errors.Join(validationErrors...) + } + + return nil +} + +func parsePolicyMappingsFile(data []byte) (*PolicyMappings, error) { + mappings := &policyMappingsFile{} + err := yaml.Unmarshal(data, mappings) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal policy mapping file: %w", err) + } + err = validateMappingsFile(mappings) + if err != nil { + return nil, fmt.Errorf("invalid policy mapping file: %w", err) + } + return expandMappingFile(mappings) +} + func LoadLocalMappings(configDir string) (*PolicyMappings, error) { if configDir == "" { return nil, nil } - mappings := &policyMappingsFile{} path := filepath.Join(configDir, MappingFilename) mappingFile, err := os.ReadFile(path) if err != nil { return nil, fmt.Errorf("failed to read local policy mapping file %s: %w", path, err) } - err = yaml.Unmarshal(mappingFile, mappings) - if err != nil { - return nil, fmt.Errorf("failed to unmarshal policy mapping file %s: %w", path, err) - } - return expandMappingFile(mappings) + return parsePolicyMappingsFile(mappingFile) } func LoadTUFMappings(tufClient tuf.Downloader, localTargetsDir string) (*PolicyMappings, error) { @@ -40,13 +89,7 @@ func LoadTUFMappings(tufClient tuf.Downloader, localTargetsDir string) (*PolicyM if err != nil { return nil, fmt.Errorf("failed to download policy mapping file %s: %w", filename, err) } - mappings := &policyMappingsFile{} - - err = yaml.Unmarshal(file.Data, mappings) - if err != nil { - return nil, fmt.Errorf("failed to unmarshal policy mapping file %s: %w", filename, err) - } - return expandMappingFile(mappings) + return parsePolicyMappingsFile(file.Data) } func expandMappingFile(mappingFile *policyMappingsFile) (*PolicyMappings, error) { diff --git a/config/config_test.go b/config/config_test.go new file mode 100644 index 0000000..d22ae11 --- /dev/null +++ b/config/config_test.go @@ -0,0 +1,82 @@ +package config + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func newMapping() *policyMappingsFile { + return &policyMappingsFile{ + Version: "v1", + Kind: "policy-mapping", + Policies: []*PolicyMapping{ + { + ID: "docker-official-images", + Files: []PolicyMappingFile{ + { + Path: "docker.io/library/alpine", + }, + }, + }, + }, + Rules: []*policyRuleFile{ + { + Pattern: "docker.io/library/alpine", + PolicyID: "docker-official-images", + }, + }, + } +} + +func TestMappingsFileValidation(t *testing.T) { + mappings := newMapping() + err := validateMappingsFile(mappings) + require.NoError(t, err) + + mappings = newMapping() + mappings.Kind = "not-policy-mapping" + err = validateMappingsFile(mappings) + require.ErrorContains(t, err, "file is not of kind policy-mapping: not-policy-mapping") + + mappings = newMapping() + mappings.Version = "v2" + err = validateMappingsFile(mappings) + require.ErrorContains(t, err, "unsupported policy mapping file version: v2") + + mappings = newMapping() + mappings.Rules[0].Pattern = "" + err = validateMappingsFile(mappings) + require.ErrorContains(t, err, "rule missing pattern") + + mappings = newMapping() + mappings.Rules[0].PolicyID = "" + err = validateMappingsFile(mappings) + require.ErrorContains(t, err, "rule must have policy-id or replacement") + + mappings = newMapping() + mappings.Rules[0].PolicyID = "docker-official-images" + mappings.Rules[0].Replacement = "docker.io/library/alpine" + err = validateMappingsFile(mappings) + require.ErrorContains(t, err, "rule cannot have both policy-id and replacement") + + mappings = newMapping() + mappings.Policies[0].ID = "" + err = validateMappingsFile(mappings) + require.ErrorContains(t, err, "policy missing id") + + mappings = newMapping() + mappings.Policies[0].Files = nil + err = validateMappingsFile(mappings) + require.ErrorContains(t, err, "policy missing files") + + mappings = newMapping() + mappings.Policies[0].Files[0].Path = "" + err = validateMappingsFile(mappings) + require.ErrorContains(t, err, "file missing path") + + // multiple errors + mappings.Policies[0].ID = "" + err = validateMappingsFile(mappings) + require.ErrorContains(t, err, "policy missing id: \nfile missing path: {}") +} diff --git a/policy/match_test.go b/policy/match_test.go index e73aa6f..b8f743d 100644 --- a/policy/match_test.go +++ b/policy/match_test.go @@ -15,10 +15,11 @@ func TestFindPolicyMatch(t *testing.T) { imageName string mappingDir string - expectError bool - expectedMatchType matchType - expectedPolicyID string - expectedImageName string + expectError bool + expectLoadingError bool + expectedMatchType matchType + expectedPolicyID string + expectedImageName string }{ { name: "alpine", @@ -79,16 +80,6 @@ func TestFindPolicyMatch(t *testing.T) { expectedPolicyID: "docker-official-images", expectedImageName: "docker.io/library/alpine", }, - { - name: "invalid rewrites", - mappingDir: "rewrite-invalid", - imageName: "mycoolmirror.org/library/alpine", - - expectError: true, - expectedMatchType: matchTypePolicy, - expectedPolicyID: "docker-official-images", - expectedImageName: "docker.io/library/alpine", - }, { name: "rewrite loop", mappingDir: "rewrite-loop",