From 33ef43065f27d515c2f52703ec6ada4f74afb75a Mon Sep 17 00:00:00 2001 From: Daniel McIlvaney Date: Mon, 30 Mar 2026 15:55:21 -0700 Subject: [PATCH 1/6] feat(config): Add fingerprint ignore tags to some config fields --- internal/projectconfig/build.go | 12 +-- internal/projectconfig/component.go | 8 +- internal/projectconfig/distro.go | 2 +- internal/projectconfig/fingerprint_test.go | 113 +++++++++++++++++++++ internal/projectconfig/overlay.go | 2 +- 5 files changed, 125 insertions(+), 12 deletions(-) create mode 100644 internal/projectconfig/fingerprint_test.go diff --git a/internal/projectconfig/build.go b/internal/projectconfig/build.go index 50ee2c7..67c7ffa 100644 --- a/internal/projectconfig/build.go +++ b/internal/projectconfig/build.go @@ -13,7 +13,7 @@ type CheckConfig struct { // Skip indicates whether the %check section should be disabled for this component. Skip bool `toml:"skip,omitempty" json:"skip,omitempty" jsonschema:"title=Skip check,description=Disables the %check section by prepending 'exit 0' when set to true"` // SkipReason provides a required justification when Skip is true. - SkipReason string `toml:"skip_reason,omitempty" json:"skipReason,omitempty" jsonschema:"title=Skip reason,description=Required justification for skipping the %check section"` + SkipReason string `toml:"skip_reason,omitempty" json:"skipReason,omitempty" jsonschema:"title=Skip reason,description=Required justification for skipping the %check section" fingerprint:"-"` } // Validate checks that required fields are set when Skip is true. @@ -43,9 +43,9 @@ type ComponentBuildConfig struct { // Check section configuration. Check CheckConfig `toml:"check,omitempty" json:"check,omitempty" jsonschema:"title=Check configuration,description=Configuration for the %check section"` // Failure configuration and policy for this component's build. - Failure ComponentBuildFailureConfig `toml:"failure,omitempty" json:"failure,omitempty" jsonschema:"title=Build failure configuration,description=Configuration and policy regarding build failures for this component."` + Failure ComponentBuildFailureConfig `toml:"failure,omitempty" json:"failure,omitempty" jsonschema:"title=Build failure configuration,description=Configuration and policy regarding build failures for this component." fingerprint:"-"` // Hints for how or when to build the component; must not be required for correctness of builds. - Hints ComponentBuildHints `toml:"hints,omitempty" json:"hints,omitempty" jsonschema:"title=Build hints,description=Non-essential hints for how or when to build the component."` + Hints ComponentBuildHints `toml:"hints,omitempty" json:"hints,omitempty" jsonschema:"title=Build hints,description=Non-essential hints for how or when to build the component." fingerprint:"-"` } // ComponentBuildFailureConfig encapsulates configuration and policy regarding a component's @@ -53,9 +53,9 @@ type ComponentBuildConfig struct { type ComponentBuildFailureConfig struct { // Expected indicates that this component is expected to fail building. This is intended to be used as a temporary // marker for components that are expected to fail until they can be fixed. - Expected bool `toml:"expected,omitempty" json:"expected,omitempty" jsonschema:"title=Expected failure,description=Indicates that this component is expected to fail building."` + Expected bool `toml:"expected,omitempty" json:"expected,omitempty" jsonschema:"title=Expected failure,description=Indicates that this component is expected to fail building." fingerprint:"-"` // ExpectedReason provides a required justification when Expected is true. - ExpectedReason string `toml:"expected-reason,omitempty" json:"expectedReason,omitempty" jsonschema:"title=Expected failure reason,description=Required justification for why this component is expected to fail building."` + ExpectedReason string `toml:"expected-reason,omitempty" json:"expectedReason,omitempty" jsonschema:"title=Expected failure reason,description=Required justification for why this component is expected to fail building." fingerprint:"-"` } // ComponentBuildHints encapsulates non-essential hints for how or when to build a component. @@ -63,7 +63,7 @@ type ComponentBuildFailureConfig struct { // or optimizations. type ComponentBuildHints struct { // Expensive indicates that building this component is relatively expensive compared to the rest of the distro. - Expensive bool `toml:"expensive,omitempty" json:"expensive,omitempty" jsonschema:"title=Expensive to build,description=Indicates that building this component is expensive and should be carefully considered when scheduling."` + Expensive bool `toml:"expensive,omitempty" json:"expensive,omitempty" jsonschema:"title=Expensive to build,description=Indicates that building this component is expensive and should be carefully considered when scheduling." fingerprint:"-"` } // Validate checks that the build configuration is valid. diff --git a/internal/projectconfig/component.go b/internal/projectconfig/component.go index 189887d..3c94573 100644 --- a/internal/projectconfig/component.go +++ b/internal/projectconfig/component.go @@ -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:"-"` } // Defines a component group. Component groups are logical groupings of components (see [ComponentConfig]). @@ -111,11 +111,11 @@ func (g ComponentGroupConfig) WithAbsolutePaths(referenceDir string) ComponentGr // Defines a component. type ComponentConfig struct { // The component's name; not actually present in serialized files. - Name string `toml:"-" json:"name" table:",sortkey"` + Name string `toml:"-" json:"name" table:",sortkey" fingerprint:"-"` // Reference to the source config file that this definition came from; not present // in serialized files. - SourceConfigFile *ConfigFile `toml:"-" json:"-" table:"-"` + SourceConfigFile *ConfigFile `toml:"-" json:"-" table:"-" fingerprint:"-"` // Where to get its spec and adjacent files from. Spec SpecSource `toml:"spec,omitempty" json:"spec,omitempty" jsonschema:"title=Spec,description=Identifies where to find the spec for this component"` diff --git a/internal/projectconfig/distro.go b/internal/projectconfig/distro.go index 5409411..89fe7e2 100644 --- a/internal/projectconfig/distro.go +++ b/internal/projectconfig/distro.go @@ -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:"-"` } // Implements the [Stringer] interface for [DistroReference]. diff --git a/internal/projectconfig/fingerprint_test.go b/internal/projectconfig/fingerprint_test.go new file mode 100644 index 0000000..c6a8b71 --- /dev/null +++ b/internal/projectconfig/fingerprint_test.go @@ -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) + } +} diff --git a/internal/projectconfig/overlay.go b/internal/projectconfig/overlay.go index 3699f73..2c603b9 100644 --- a/internal/projectconfig/overlay.go +++ b/internal/projectconfig/overlay.go @@ -17,7 +17,7 @@ type ComponentOverlay struct { // The type of overlay to apply. Type ComponentOverlayType `toml:"type" json:"type" validate:"required" jsonschema:"enum=spec-add-tag,enum=spec-insert-tag,enum=spec-set-tag,enum=spec-update-tag,enum=spec-remove-tag,enum=spec-prepend-lines,enum=spec-append-lines,enum=spec-search-replace,enum=spec-remove-section,enum=patch-add,enum=patch-remove,enum=file-prepend-lines,enum=file-search-replace,enum=file-add,enum=file-remove,enum=file-rename,title=Overlay type,description=The type of overlay to apply"` // Human readable description of overlay; primarily present to document the need for the change. - Description string `toml:"description,omitempty" json:"description,omitempty" jsonschema:"title=Description,description=Human readable description of overlay"` + Description string `toml:"description,omitempty" json:"description,omitempty" jsonschema:"title=Description,description=Human readable description of overlay" fingerprint:"-"` // For overlays that apply to non-spec files, indicates the filename. For overlays that can // apply to multiple files, supports glob patterns (including globstar). From 4b0d14ebc640d708663d7e858f0023e0acfdc2e2 Mon Sep 17 00:00:00 2001 From: Daniel McIlvaney Date: Wed, 1 Apr 2026 16:30:33 -0700 Subject: [PATCH 2/6] remove redundant flags --- internal/projectconfig/build.go | 6 +++--- internal/projectconfig/fingerprint_test.go | 13 ------------- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/internal/projectconfig/build.go b/internal/projectconfig/build.go index 67c7ffa..ac144ab 100644 --- a/internal/projectconfig/build.go +++ b/internal/projectconfig/build.go @@ -53,9 +53,9 @@ type ComponentBuildConfig struct { type ComponentBuildFailureConfig struct { // Expected indicates that this component is expected to fail building. This is intended to be used as a temporary // marker for components that are expected to fail until they can be fixed. - Expected bool `toml:"expected,omitempty" json:"expected,omitempty" jsonschema:"title=Expected failure,description=Indicates that this component is expected to fail building." fingerprint:"-"` + Expected bool `toml:"expected,omitempty" json:"expected,omitempty" jsonschema:"title=Expected failure,description=Indicates that this component is expected to fail building."` // ExpectedReason provides a required justification when Expected is true. - ExpectedReason string `toml:"expected-reason,omitempty" json:"expectedReason,omitempty" jsonschema:"title=Expected failure reason,description=Required justification for why this component is expected to fail building." fingerprint:"-"` + ExpectedReason string `toml:"expected-reason,omitempty" json:"expectedReason,omitempty" jsonschema:"title=Expected failure reason,description=Required justification for why this component is expected to fail building."` } // ComponentBuildHints encapsulates non-essential hints for how or when to build a component. @@ -63,7 +63,7 @@ type ComponentBuildFailureConfig struct { // or optimizations. type ComponentBuildHints struct { // Expensive indicates that building this component is relatively expensive compared to the rest of the distro. - Expensive bool `toml:"expensive,omitempty" json:"expensive,omitempty" jsonschema:"title=Expensive to build,description=Indicates that building this component is expensive and should be carefully considered when scheduling." fingerprint:"-"` + Expensive bool `toml:"expensive,omitempty" json:"expensive,omitempty" jsonschema:"title=Expensive to build,description=Indicates that building this component is expensive and should be carefully considered when scheduling."` } // Validate checks that the build configuration is valid. diff --git a/internal/projectconfig/fingerprint_test.go b/internal/projectconfig/fingerprint_test.go index c6a8b71..adaaa16 100644 --- a/internal/projectconfig/fingerprint_test.go +++ b/internal/projectconfig/fingerprint_test.go @@ -27,8 +27,6 @@ func TestAllFingerprintedFieldsHaveDecision(t *testing.T) { 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](), @@ -54,17 +52,6 @@ func TestAllFingerprintedFieldsHaveDecision(t *testing.T) { // 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, From 04616715ad90c839f14197f6de533a6f3d79aafb Mon Sep 17 00:00:00 2001 From: Daniel McIlvaney Date: Wed, 1 Apr 2026 16:38:24 -0700 Subject: [PATCH 3/6] don't include publish details --- internal/projectconfig/fingerprint_test.go | 4 ++++ internal/projectconfig/package.go | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/projectconfig/fingerprint_test.go b/internal/projectconfig/fingerprint_test.go index adaaa16..5a77f9e 100644 --- a/internal/projectconfig/fingerprint_test.go +++ b/internal/projectconfig/fingerprint_test.go @@ -27,6 +27,7 @@ func TestAllFingerprintedFieldsHaveDecision(t *testing.T) { reflect.TypeFor[projectconfig.ComponentConfig](), reflect.TypeFor[projectconfig.ComponentBuildConfig](), reflect.TypeFor[projectconfig.CheckConfig](), + reflect.TypeFor[projectconfig.PackageConfig](), reflect.TypeFor[projectconfig.ComponentOverlay](), reflect.TypeFor[projectconfig.SpecSource](), reflect.TypeFor[projectconfig.DistroReference](), @@ -52,6 +53,9 @@ func TestAllFingerprintedFieldsHaveDecision(t *testing.T) { // CheckConfig.SkipReason — human documentation for why check is skipped, not a build input. "CheckConfig.SkipReason": true, + // PackageConfig.Publish — post-build routing (where to publish), not a build input. + "PackageConfig.Publish": true, + // ComponentOverlay.Description — human-readable documentation for the overlay. "ComponentOverlay.Description": true, diff --git a/internal/projectconfig/package.go b/internal/projectconfig/package.go index 25756fb..6eca79c 100644 --- a/internal/projectconfig/package.go +++ b/internal/projectconfig/package.go @@ -24,7 +24,7 @@ type PackagePublishConfig struct { // Currently only publish settings are supported; additional fields may be added in the future. type PackageConfig struct { // Publish holds the publish settings for this package. - Publish PackagePublishConfig `toml:"publish,omitempty" json:"publish,omitempty" jsonschema:"title=Publish settings,description=Publishing settings for this binary package"` + Publish PackagePublishConfig `toml:"publish,omitempty" json:"publish,omitempty" jsonschema:"title=Publish settings,description=Publishing settings for this binary package" fingerprint:"-"` } // MergeUpdatesFrom updates the package config with non-zero values from other. From 352790bc177dc28556c9348cc65981fe408fea88 Mon Sep 17 00:00:00 2001 From: Daniel McIlvaney Date: Wed, 1 Apr 2026 16:41:14 -0700 Subject: [PATCH 4/6] test for bad fingerprint values --- internal/projectconfig/fingerprint_test.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/internal/projectconfig/fingerprint_test.go b/internal/projectconfig/fingerprint_test.go index 5a77f9e..4624864 100644 --- a/internal/projectconfig/fingerprint_test.go +++ b/internal/projectconfig/fingerprint_test.go @@ -73,7 +73,7 @@ func TestAllFingerprintedFieldsHaveDecision(t *testing.T) { "SourceFileReference.Origin": true, } - // Collect all actual exclusions found via reflection. + // Collect all actual exclusions found via reflection, and flag invalid tag values. actualExclusions := make(map[string]bool) for _, st := range fingerprintedStructs { @@ -82,8 +82,20 @@ func TestAllFingerprintedFieldsHaveDecision(t *testing.T) { key := st.Name() + "." + field.Name tag := field.Tag.Get("fingerprint") - if tag == "-" { + + switch tag { + case "": + // No tag — included by default (the safe default). + case "-": actualExclusions[key] = true + default: + // hashstructure only recognises "" (include) and "-" (exclude). + // Any other value is silently treated as included, which is + // almost certainly a typo. + assert.Failf(t, "invalid fingerprint tag", + "field %q has unrecognised fingerprint tag value %q — "+ + "only `fingerprint:\"-\"` (exclude) is valid; "+ + "remove the tag to include the field", key, tag) } } } From be4cdc71eed116c96e38636535d9b56346633697 Mon Sep 17 00:00:00 2001 From: Daniel McIlvaney Date: Wed, 1 Apr 2026 17:48:52 -0700 Subject: [PATCH 5/6] remove redundant type load --- internal/projectconfig/fingerprint_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/projectconfig/fingerprint_test.go b/internal/projectconfig/fingerprint_test.go index 4624864..b3bcb86 100644 --- a/internal/projectconfig/fingerprint_test.go +++ b/internal/projectconfig/fingerprint_test.go @@ -32,7 +32,6 @@ func TestAllFingerprintedFieldsHaveDecision(t *testing.T) { 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 From d29895426d997a6dd2e07a05158118d66ccdaac7 Mon Sep 17 00:00:00 2001 From: Daniel McIlvaney Date: Wed, 1 Apr 2026 18:57:45 -0700 Subject: [PATCH 6/6] add instructions file --- .github/instructions/go.instructions.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/instructions/go.instructions.md b/.github/instructions/go.instructions.md index ef82956..482939b 100644 --- a/.github/instructions/go.instructions.md +++ b/.github/instructions/go.instructions.md @@ -83,6 +83,18 @@ applyTo: "**/*.go" } ``` +## Component Fingerprinting — `fingerprint:"-"` Tags + +Structs in `internal/projectconfig/` are hashed by `hashstructure.Hash()` to detect component changes. Fields **included by default** (safe: false positive > false negative). + +When adding a new field to a fingerprinted struct, ask: **"Does changing this field change the build output?"** +- **Yes** (build flags, spec source, defines, etc.) → do nothing, included automatically. +- **No** (human docs, scheduling hints, CI policy, metadata, back-references) → add `fingerprint:"-"` to the struct tag and register the exclusion in `expectedExclusions` in `internal/projectconfig/fingerprint_test.go`. + +If a parent struct field is already excluded (e.g. `Failure ComponentBuildFailureConfig ... fingerprint:"-"`), do **not** also tag the inner struct's fields — `hashstructure` skips the entire subtree. + +Run `mage unit` to verify — the guard test will catch unregistered exclusions or missing tags. + ## Quality Standards - Make minimal, focused changes to achieve the required functionality