-
Notifications
You must be signed in to change notification settings - Fork 6
feat(config): Add fingerprint ignore tags to some config fields #46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -49,7 +49,7 @@ type Origin struct { | |||||
| // SourceFileReference encapsulates a reference to a specific source file artifact. | ||||||
| type SourceFileReference struct { | ||||||
| // Reference to the component to which the source file belongs. | ||||||
| Component ComponentReference `toml:"-" json:"-"` | ||||||
| Component ComponentReference `toml:"-" json:"-" fingerprint:"-"` | ||||||
|
|
||||||
| // Name of the source file; must be non-empty. | ||||||
| Filename string `toml:"filename" json:"filename"` | ||||||
|
|
@@ -61,7 +61,7 @@ type SourceFileReference struct { | |||||
| HashType fileutils.HashType `toml:"hash-type,omitempty" json:"hashType,omitempty"` | ||||||
|
|
||||||
| // Origin for this source file. When omitted, the file is resolved via the lookaside cache. | ||||||
| Origin Origin `toml:"origin,omitempty" json:"origin,omitempty"` | ||||||
| Origin Origin `toml:"origin,omitempty" json:"origin,omitempty" fingerprint:"-"` | ||||||
|
||||||
| Origin Origin `toml:"origin,omitempty" json:"origin,omitempty" fingerprint:"-"` | |
| Origin Origin `toml:"origin,omitempty" json:"origin,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will add a check to the calculation step that errors on this case
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -18,7 +18,7 @@ type DistroReference struct { | |||||
| // Version of the referenced distro. | ||||||
| Version string `toml:"version,omitempty" json:"version,omitempty" jsonschema:"title=Version,description=Version of the referenced distro"` | ||||||
| // Snapshot date/time for source code if specified components will use source as it existed at this time. | ||||||
| Snapshot string `toml:"snapshot,omitempty" json:"snapshot,omitempty" jsonschema:"format=date-time,title=Snapshot,description=If specified use source code as it existed at this date/time"` | ||||||
| Snapshot string `toml:"snapshot,omitempty" json:"snapshot,omitempty" jsonschema:"format=date-time,title=Snapshot,description=If specified use source code as it existed at this date/time" fingerprint:"-"` | ||||||
|
||||||
| Snapshot string `toml:"snapshot,omitempty" json:"snapshot,omitempty" jsonschema:"format=date-time,title=Snapshot,description=If specified use source code as it existed at this date/time" fingerprint:"-"` | |
| Snapshot string `toml:"snapshot,omitempty" json:"snapshot,omitempty" jsonschema:"format=date-time,title=Snapshot,description=If specified use source code as it existed at this date/time"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package projectconfig_test | ||
|
|
||
| import ( | ||
| "reflect" | ||
| "testing" | ||
|
|
||
| "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" | ||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| // TestAllFingerprintedFieldsHaveDecision verifies that every field in every | ||
| // fingerprinted struct has been consciously categorized as either included | ||
| // (no fingerprint tag) or excluded (`fingerprint:"-"`). | ||
| // | ||
| // This test serves two purposes: | ||
| // 1. It ensures that newly added fields default to **included** in the fingerprint | ||
| // (the safe default — you get a false positive, never a false negative). | ||
| // 2. It catches accidental removal of `fingerprint:"-"` tags from excluded fields, | ||
| // since all exclusions are tracked in expectedExclusions. | ||
| func TestAllFingerprintedFieldsHaveDecision(t *testing.T) { | ||
| // All struct types whose fields participate in component fingerprinting. | ||
| // When adding a new struct that feeds into the fingerprint, add it here. | ||
| fingerprintedStructs := []reflect.Type{ | ||
| reflect.TypeFor[projectconfig.ComponentConfig](), | ||
| reflect.TypeFor[projectconfig.ComponentBuildConfig](), | ||
| reflect.TypeFor[projectconfig.CheckConfig](), | ||
| reflect.TypeFor[projectconfig.ComponentBuildFailureConfig](), | ||
| reflect.TypeFor[projectconfig.ComponentBuildHints](), | ||
| reflect.TypeFor[projectconfig.ComponentOverlay](), | ||
| reflect.TypeFor[projectconfig.SpecSource](), | ||
| reflect.TypeFor[projectconfig.DistroReference](), | ||
| reflect.TypeFor[projectconfig.SourceFileReference](), | ||
| reflect.TypeFor[projectconfig.Origin](), | ||
| } | ||
|
|
||
| // Maps "StructName.FieldName" for every field that should carry a | ||
| // `fingerprint:"-"` tag. Catches accidental tag removal. | ||
| // | ||
| // Each entry documents WHY the field is excluded from the fingerprint: | ||
| expectedExclusions := map[string]bool{ | ||
| // ComponentConfig.Name — metadata, already the map key in project config. | ||
| "ComponentConfig.Name": true, | ||
| // ComponentConfig.SourceConfigFile — internal bookkeeping reference, not a build input. | ||
| "ComponentConfig.SourceConfigFile": true, | ||
|
|
||
| // ComponentBuildConfig.Failure — CI policy (expected failure tracking), not a build input. | ||
| "ComponentBuildConfig.Failure": true, | ||
| // ComponentBuildConfig.Hints — scheduling hints (e.g. expensive), not a build input. | ||
| "ComponentBuildConfig.Hints": true, | ||
|
|
||
| // CheckConfig.SkipReason — human documentation for why check is skipped, not a build input. | ||
| "CheckConfig.SkipReason": true, | ||
|
|
||
| // ComponentBuildFailureConfig — entire struct excluded via parent, but individual | ||
| // fields are also tagged so reflection on the struct alone is consistent. | ||
| // ComponentBuildFailureConfig.Expected — CI decision about expected failures. | ||
| "ComponentBuildFailureConfig.Expected": true, | ||
| // ComponentBuildFailureConfig.ExpectedReason — documentation for expected failure. | ||
| "ComponentBuildFailureConfig.ExpectedReason": true, | ||
|
|
||
| // ComponentBuildHints — entire struct excluded via parent, fields also tagged. | ||
| // ComponentBuildHints.Expensive — scheduling hint, does not affect build output. | ||
| "ComponentBuildHints.Expensive": true, | ||
|
|
||
| // ComponentOverlay.Description — human-readable documentation for the overlay. | ||
| "ComponentOverlay.Description": true, | ||
|
|
||
| // SourceFileReference.Component — back-reference to parent, not a build input. | ||
| "SourceFileReference.Component": true, | ||
|
|
||
| // DistroReference.Snapshot — snapshot timestamp is not a build input; the resolved | ||
| // upstream commit hash (captured separately via SourceIdentity) is what matters. | ||
| // Excluding this prevents a snapshot bump from marking all upstream components as changed. | ||
| "DistroReference.Snapshot": true, | ||
|
|
||
| // SourceFileReference.Origin — download location metadata (URI, type), not a build input. | ||
| // The file content is already captured by Filename + Hash; changing a CDN URL should not | ||
| // trigger a rebuild. | ||
| "SourceFileReference.Origin": true, | ||
| } | ||
|
|
||
| // Collect all actual exclusions found via reflection. | ||
| actualExclusions := make(map[string]bool) | ||
|
|
||
| for _, st := range fingerprintedStructs { | ||
| for i := range st.NumField() { | ||
| field := st.Field(i) | ||
| key := st.Name() + "." + field.Name | ||
|
|
||
| tag := field.Tag.Get("fingerprint") | ||
| if tag == "-" { | ||
| actualExclusions[key] = true | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Verify every expected exclusion is actually present. | ||
| for key := range expectedExclusions { | ||
| assert.Truef(t, actualExclusions[key], | ||
| "expected field %q to have `fingerprint:\"-\"` tag, but it does not — "+ | ||
| "was the tag accidentally removed?", key) | ||
| } | ||
|
|
||
| // Verify no unexpected exclusions exist. | ||
| for key := range actualExclusions { | ||
| assert.Truef(t, expectedExclusions[key], | ||
| "field %q has `fingerprint:\"-\"` tag but is not in expectedExclusions — "+ | ||
| "add it to expectedExclusions if the exclusion is intentional", key) | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.