Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions internal/projectconfig/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -43,27 +43,27 @@ 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
// build failure.
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.
// These are not required for correctness of builds, but may be used by tools to provide guidance
// 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.
Expand Down
8 changes: 4 additions & 4 deletions internal/projectconfig/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand All @@ -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:"-"`
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excluding SourceFileReference.Origin from the fingerprint assumes the source file’s content is fully captured by Filename + Hash/HashType, but hash/hash-type are optional and downloads proceed without validation when they’re empty (see downloadAndValidate in internal/providers/sourceproviders/sourcemanager.go). If the fingerprint is used to decide whether sources/build outputs are up-to-date, consider including Origin when hash info is missing, or enforce that hash + hash-type are required for source-files entries so changes in origin can’t silently change inputs without changing the fingerprint.

Suggested change
Origin Origin `toml:"origin,omitempty" json:"origin,omitempty" fingerprint:"-"`
Origin Origin `toml:"origin,omitempty" json:"origin,omitempty"`

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

Copy link
Copy Markdown
Contributor Author

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

}

// Defines a component group. Component groups are logical groupings of components (see [ComponentConfig]).
Expand Down Expand Up @@ -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"`
Expand Down
2 changes: 1 addition & 1 deletion internal/projectconfig/distro.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:"-"`
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DistroReference.Snapshot directly affects which upstream commit is checked out when upstream-commit isn’t set (see checkoutTargetCommit in internal/providers/sourceproviders/fedorasourceprovider.go). Ignoring snapshot in the fingerprint can therefore miss real input changes when the snapshot time changes. If you intend to keep snapshot out of the fingerprint, the resolved commit hash used for the checkout should be captured elsewhere and included in the fingerprint/state instead.

Suggested change
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"`

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

}

// Implements the [Stringer] interface for [DistroReference].
Expand Down
113 changes: 113 additions & 0 deletions internal/projectconfig/fingerprint_test.go
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)
}
}
2 changes: 1 addition & 1 deletion internal/projectconfig/overlay.go
Original file line number Diff line number Diff line change
Expand Up @@ -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=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).
Expand Down
Loading