diff --git a/docs/user/reference/cli/azldev_component_build.md b/docs/user/reference/cli/azldev_component_build.md index bc475875..8ee2e16b 100644 --- a/docs/user/reference/cli/azldev_component_build.md +++ b/docs/user/reference/cli/azldev_component_build.md @@ -57,6 +57,7 @@ azldev component build [flags] --mock-config-opt stringToString Pass a configuration option through to mock (key=value, can be specified multiple times) (default []) --no-check Skip package %check tests --preserve-buildenv policy Preserve build environment {on-failure, always, never} (default on-failure) + --skip-lock-validation skip lock file consistency checks (default true) -s, --spec-path stringArray Spec path --srpm-only Build SRPM (source RPM) *only* --with-git Create a dist-git repository with synthetic commit history (requires a project git repository) diff --git a/docs/user/reference/cli/azldev_component_diff-sources.md b/docs/user/reference/cli/azldev_component_diff-sources.md index 34969640..8884d91c 100644 --- a/docs/user/reference/cli/azldev_component_diff-sources.md +++ b/docs/user/reference/cli/azldev_component_diff-sources.md @@ -22,6 +22,7 @@ azldev component diff-sources [flags] -g, --component-group stringArray Component group name -h, --help help for diff-sources --output-file string write the diff output to a file instead of stdout + --skip-lock-validation skip lock file consistency checks (default true) -s, --spec-path stringArray Spec path ``` diff --git a/docs/user/reference/cli/azldev_component_list.md b/docs/user/reference/cli/azldev_component_list.md index 2900def0..3c908a2a 100644 --- a/docs/user/reference/cli/azldev_component_list.md +++ b/docs/user/reference/cli/azldev_component_list.md @@ -38,6 +38,7 @@ azldev component list [flags] -p, --component stringArray Component name pattern -g, --component-group stringArray Component group name -h, --help help for list + --skip-lock-validation skip lock file consistency checks (default true) -s, --spec-path stringArray Spec path ``` diff --git a/docs/user/reference/cli/azldev_component_prepare-sources.md b/docs/user/reference/cli/azldev_component_prepare-sources.md index 9c3f442c..64cfa48d 100644 --- a/docs/user/reference/cli/azldev_component_prepare-sources.md +++ b/docs/user/reference/cli/azldev_component_prepare-sources.md @@ -39,6 +39,7 @@ azldev component prepare-sources [flags] --force delete and recreate the output directory if it already exists -h, --help help for prepare-sources -o, --output-dir string output directory + --skip-lock-validation skip lock file consistency checks (default true) --skip-overlays skip applying overlays to prepared sources -s, --spec-path stringArray Spec path --with-git Create a dist-git repository with synthetic commit history (requires a project git repository) diff --git a/docs/user/reference/cli/azldev_component_query.md b/docs/user/reference/cli/azldev_component_query.md index 26bd9750..c1604a9e 100644 --- a/docs/user/reference/cli/azldev_component_query.md +++ b/docs/user/reference/cli/azldev_component_query.md @@ -34,6 +34,7 @@ azldev component query [flags] -p, --component stringArray Component name pattern -g, --component-group stringArray Component group name -h, --help help for query + --skip-lock-validation skip lock file consistency checks (default true) -s, --spec-path stringArray Spec path ``` diff --git a/docs/user/reference/cli/azldev_component_render.md b/docs/user/reference/cli/azldev_component_render.md index 2da5f357..aba335fa 100644 --- a/docs/user/reference/cli/azldev_component_render.md +++ b/docs/user/reference/cli/azldev_component_render.md @@ -20,6 +20,11 @@ Unlike prepare-sources, render skips downloading source tarballs from the lookaside cache — only spec files, patches, scripts, and other git-tracked sidecar files are included. Multiple components can be rendered at once. +When rendering all components (-a), the --clean-stale flag removes rendered +directories that no longer correspond to any current component. Stale cleanup +is skipped when rendering individual components to avoid accidentally removing +directories for components not included in the filter. + ``` azldev component render [flags] ``` @@ -51,6 +56,7 @@ azldev component render [flags] -f, --force allow overwriting existing rendered component directories -h, --help help for render -o, --output-dir string output directory for rendered specs (overrides rendered-specs-dir from config) + --skip-lock-validation skip lock file consistency checks (default true) -s, --spec-path stringArray Spec path ``` diff --git a/docs/user/reference/cli/azldev_component_update.md b/docs/user/reference/cli/azldev_component_update.md index a8a0a7e7..fa365e33 100644 --- a/docs/user/reference/cli/azldev_component_update.md +++ b/docs/user/reference/cli/azldev_component_update.md @@ -15,6 +15,11 @@ reproducible results. Local components are skipped — they have no upstream commit to resolve. +When updating all components (-a), orphan lock files (locks for components +that no longer exist in the project config) are automatically pruned. +Orphan pruning is skipped when updating individual components to avoid +accidentally removing lock files for components not included in the filter. + ``` azldev component update [flags] ``` @@ -39,6 +44,7 @@ azldev component update [flags] -p, --component stringArray Component name pattern -g, --component-group stringArray Component group name -h, --help help for update + --skip-lock-validation skip lock file consistency checks (default true) -s, --spec-path stringArray Spec path ``` diff --git a/internal/app/azldev/app.go b/internal/app/azldev/app.go index 49f8f22c..2f7a1bc8 100644 --- a/internal/app/azldev/app.go +++ b/internal/app/azldev/app.go @@ -157,6 +157,13 @@ lives), or use -C to point to one.`, app.cmd.SetCompletionCommandGroupID(CommandGroupMeta) app.addAdvancedCommandHint() + app.registerGlobalFlags() + + return app +} + +// registerGlobalFlags defines all persistent (global) flags for the CLI. +func (app *App) registerGlobalFlags() { // Define global flags and configuration settings. app.cmd.PersistentFlags().BoolVarP(&app.verbose, "verbose", "v", false, "enable verbose output") app.cmd.PersistentFlags().BoolVarP(&app.quiet, "quiet", "q", false, "only enable minimal output") @@ -176,8 +183,6 @@ lives), or use -C to point to one.`, "output colorization mode {always, auto, never}") app.cmd.PersistentFlags().BoolVar(&app.permissiveConfigParsing, "permissive-config", false, "do not fail on unknown fields in TOML config files") - - return app } // addAdvancedCommandHint embeds a hint about the hidden "advanced" command group diff --git a/internal/app/azldev/cmds/component/list.go b/internal/app/azldev/cmds/component/list.go index 6b0ee4f2..3671b16d 100644 --- a/internal/app/azldev/cmds/component/list.go +++ b/internal/app/azldev/cmds/component/list.go @@ -48,6 +48,10 @@ Component name patterns support glob syntax (*, ?, []).`, RunE: azldev.RunFuncWithExtraArgs(func(env *azldev.Env, args []string) (interface{}, error) { options.ComponentFilter.ComponentNamePatterns = append(args, options.ComponentFilter.ComponentNamePatterns...) + // List is read-only — skip lock validation so users can always + // inspect their components even when locks are stale. + options.ComponentFilter.SkipLockValidation = true + return ListComponentConfigs(env, options) }), ValidArgsFunction: components.GenerateComponentNameCompletions, diff --git a/internal/app/azldev/cmds/component/render.go b/internal/app/azldev/cmds/component/render.go index 20126af2..e0f0240b 100644 --- a/internal/app/azldev/cmds/component/render.go +++ b/internal/app/azldev/cmds/component/render.go @@ -57,7 +57,12 @@ specs/v/vim). Unlike prepare-sources, render skips downloading source tarballs from the lookaside cache — only spec files, patches, scripts, and other git-tracked -sidecar files are included. Multiple components can be rendered at once.`, +sidecar files are included. Multiple components can be rendered at once. + +When rendering all components (-a), the --clean-stale flag removes rendered +directories that no longer correspond to any current component. Stale cleanup +is skipped when rendering individual components to avoid accidentally removing +directories for components not included in the filter.`, Example: ` # Render all components (output dir from config) azldev component render -a diff --git a/internal/app/azldev/cmds/component/update.go b/internal/app/azldev/cmds/component/update.go index 7883e833..16a206a6 100644 --- a/internal/app/azldev/cmds/component/update.go +++ b/internal/app/azldev/cmds/component/update.go @@ -41,7 +41,12 @@ distro snapshot time or explicit pin, then records it in locks/.lock. Subsequent commands (render, build) use the locked commit for deterministic, reproducible results. -Local components are skipped — they have no upstream commit to resolve.`, +Local components are skipped — they have no upstream commit to resolve. + +When updating all components (-a), orphan lock files (locks for components +that no longer exist in the project config) are automatically pruned. +Orphan pruning is skipped when updating individual components to avoid +accidentally removing lock files for components not included in the filter.`, Example: ` # Update all components azldev component update -a @@ -51,6 +56,9 @@ Local components are skipped — they have no upstream commit to resolve.`, # Update components in a group azldev component update -g core`, RunE: azldev.RunFuncWithExtraArgs(func(env *azldev.Env, args []string) (interface{}, error) { + // Skip lock validation — update is the lock file writer. + options.ComponentFilter.SkipLockValidation = true + options.ComponentFilter.ComponentNamePatterns = append( args, options.ComponentFilter.ComponentNamePatterns..., ) @@ -87,11 +95,11 @@ func UpdateComponents(env *azldev.Env, options *UpdateComponentOptions) ([]Updat } comps := resolved.Components() - if len(comps) == 0 { + if len(comps) == 0 && !options.ComponentFilter.IncludeAllComponents { return nil, errors.New("no components matched the filter") } - // Resolve upstream commits in parallel. + // Resolve upstream commits in parallel (no-op for empty list). store := env.LockStore() if store == nil { return nil, errors.New("no project directory configured; cannot update lock files") @@ -99,8 +107,13 @@ func UpdateComponents(env *azldev.Env, options *UpdateComponentOptions) ([]Updat results := resolveUpstreamCommitsParallel(env, comps, store) - // Check results and bail on errors/cancellation before saving. - if err := checkUpdateResults(env, results); err != nil { + // Don't save if the context was cancelled (Ctrl+C). + if env.Context().Err() != nil { + return results, errors.New("update cancelled; lock files not updated") + } + + // Check results and bail on errors before saving. + if err := checkUpdateResults(results); err != nil { return results, err } @@ -109,8 +122,33 @@ func UpdateComponents(env *azldev.Env, options *UpdateComponentOptions) ([]Updat return results, err } - // Filter results for table output: only show changed components. - return filterChangedResults(results), nil + // Prune orphan lock files when updating all components. + // Use the resolved component set (not raw config) to include + // spec-glob-discovered components that aren't in config directly. + // Lock files are version controlled, so pruning is safe even if the + // resolved set is empty (e.g., all components removed from config). + if options.ComponentFilter.IncludeAllComponents { + if len(comps) == 0 { + slog.Warn("No components resolved; all existing lock files will be treated as orphans") + } + + resolvedNames := make(map[string]projectconfig.ComponentConfig, len(comps)) + for _, comp := range comps { + resolvedNames[comp.GetName()] = *comp.GetConfig() + } + + pruned, pruneErr := store.PruneOrphans(resolvedNames) + if pruneErr != nil { + return results, fmt.Errorf("pruning orphan lock files:\n%w", pruneErr) + } + + if pruned > 0 { + slog.Info("Pruned orphan lock files", "count", pruned) + } + } + + // Filter results for table output: show changed and skipped components. + return filterDisplayResults(results), nil } // saveComponentLocks writes a lock file for each changed component. @@ -144,8 +182,8 @@ func saveComponentLocks(store *lockfile.Store, results []UpdateResult) error { } // checkUpdateResults counts results, logs a summary, and returns an error if any -// component failed or the context was cancelled. -func checkUpdateResults(env *azldev.Env, results []UpdateResult) error { +// component failed. +func checkUpdateResults(results []UpdateResult) error { var changed, skipped int var failedNames []string @@ -171,10 +209,6 @@ func checkUpdateResults(env *azldev.Env, results []UpdateResult) error { len(failedNames), strings.Join(failedNames, "\n ")) } - if env.Context().Err() != nil { - return errors.New("update cancelled; lock files not updated") - } - slog.Info("Update complete", "total", len(results), "changed", changed, @@ -183,12 +217,12 @@ func checkUpdateResults(env *azldev.Env, results []UpdateResult) error { return nil } -// filterChangedResults returns only changed results for table display. -func filterChangedResults(results []UpdateResult) []UpdateResult { +// filterDisplayResults returns changed and skipped results for table display. +func filterDisplayResults(results []UpdateResult) []UpdateResult { var tableResults []UpdateResult for idx := range results { - if results[idx].Changed { + if results[idx].Changed || results[idx].Skipped { tableResults = append(tableResults, results[idx]) } } @@ -263,7 +297,7 @@ func resolveUpstreamCommitsParallel( // checkLockChanged compares the resolved commit against the existing lock file // to determine if the component changed. Distinguishes "not found" (new -// component) from real errors (corrupt lock file). +// component) from real errors (corrupt/unreadable lock file). func checkLockChanged(store *lockfile.Store, componentName string, result *UpdateResult) { exists, existsErr := store.Exists(componentName) if existsErr != nil { diff --git a/internal/app/azldev/core/components/filter.go b/internal/app/azldev/core/components/filter.go index a015f20e..678620cb 100644 --- a/internal/app/azldev/core/components/filter.go +++ b/internal/app/azldev/core/components/filter.go @@ -4,6 +4,7 @@ package components import ( + "os" "strings" "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev" @@ -22,6 +23,13 @@ type ComponentFilter struct { SpecPaths []string // If true, then *all* known components are included in the result set. IncludeAllComponents bool + // SkipLockValidation disables lock file consistency checks for this + // filter's resolution. Commands that write lock files (update) or are + // read-only (list) set this to true. The '--skip-lock-validation' flag + // defaults based on AZLDEV_ENABLE_LOCK_VALIDATION during rollout. + //nolint:godox // tracked by TODO(lockfiles) tag. + // TODO(lockfiles): remove env var gate; default to false (validation on). + SkipLockValidation bool } // HasNoCriteria returns true if the filter has no criteria set, meaning that it will never @@ -48,6 +56,10 @@ func AddComponentFilterOptionsToCommand(cmd *cobra.Command, filter *ComponentFil cmd.Flags().StringArrayVarP(&filter.SpecPaths, "spec-path", "s", []string{}, "Spec path") _ = cmd.MarkFlagFilename("spec-path", ".spec") + + cmd.Flags().BoolVar(&filter.SkipLockValidation, "skip-lock-validation", + os.Getenv("AZLDEV_ENABLE_LOCK_VALIDATION") != "1", + "skip lock file consistency checks") } // Function suitable for use as a [cobra.ValidArgsFunction] in a [cobra.Command]. Intended for use diff --git a/internal/app/azldev/core/components/resolver.go b/internal/app/azldev/core/components/resolver.go index 124608da..d959863e 100644 --- a/internal/app/azldev/core/components/resolver.go +++ b/internal/app/azldev/core/components/resolver.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "log/slog" + "os" "path" "path/filepath" "strings" @@ -34,6 +35,15 @@ func NewResolver(env *azldev.Env) *Resolver { // Given a component filter, finds all components defined in the environment that match the filter. func (r *Resolver) FindComponents(filter *ComponentFilter) (components *ComponentSet, err error) { + // The filter's SkipLockValidation field is the primary control. When + // created via AddComponentFilterOptionsToCommand, its default comes from + // the AZLDEV_ENABLE_LOCK_VALIDATION env var. For programmatic callers + // (including tests), also check the env var here so the rollout gate + // applies uniformly. + //nolint:godox // tracked by TODO(lockfiles) tag. + // TODO(lockfiles): remove env var gate; filter.SkipLockValidation becomes sole control. + skipValidation := filter.SkipLockValidation || os.Getenv("AZLDEV_ENABLE_LOCK_VALIDATION") != "1" + // For usability's sake, detect if the caller/user forgot to specify *any* criteria. if filter.HasNoCriteria() { slog.Warn("No component selection options were given, no components will be selected.") @@ -43,7 +53,12 @@ func (r *Resolver) FindComponents(filter *ComponentFilter) (components *Componen // If we were asked to include all components, then it's not even worth looking at anything else. if filter.IncludeAllComponents { - return r.FindAllComponents() + allComps, findErr := r.FindAllComponents() + if findErr != nil { + return allComps, findErr + } + + return allComps, r.validateLockFiles(allComps, true, skipValidation) } components = NewComponentSet() @@ -72,7 +87,7 @@ func (r *Resolver) FindComponents(filter *ComponentFilter) (components *Componen } } - return components, err + return components, r.validateLockFiles(components, false, skipValidation) } // Finds *all* components defined in the environment. @@ -493,3 +508,47 @@ func applyInheritedDefaultsToComponent( return &resolved, nil } + +// validateLockFiles checks lock file consistency against the resolved component +// set. Skipped when skipValidation is true (set per-filter or via the global +// '--skip-lock-validation' flag). +// +// When checkOrphans is true (i.e., all components are being validated), orphan +// lock files are also detected. On filtered commands, only missing/stale checks +// run — orphan detection is a project-wide invariant that would misfire against +// a subset. +func (r *Resolver) validateLockFiles(resolved *ComponentSet, checkOrphans bool, skipValidation bool) error { + if skipValidation { + return nil + } + + reader := r.env.LockReader() + if reader == nil { + return nil + } + + // Build resolved config map from the component set. + resolvedConfigs := make(map[string]projectconfig.ComponentConfig, resolved.Len()) + for _, comp := range resolved.Components() { + resolvedConfigs[comp.GetName()] = *comp.GetConfig() + } + + stale, orphans, err := reader.ValidateConsistency(resolvedConfigs, checkOrphans) + if err == nil { + return nil + } + + // Format fix suggestions at the call site (not in the lockfile package) + // so CLI-specific strings don't leak into the data layer. + const maxIssuesForDetailedSuggestion = 10 + + if len(orphans) > 0 || len(stale) > maxIssuesForDetailedSuggestion { + r.env.AddFixSuggestion("run 'azldev component update -a' to fix all lock file issues") + } else if len(stale) > 0 { + r.env.AddFixSuggestion(fmt.Sprintf( + "run 'azldev component update %s'", + strings.Join(stale, " "))) + } + + return fmt.Errorf("lock file validation failed:\n%w", err) +} diff --git a/internal/lockfile/lockfile.go b/internal/lockfile/lockfile.go index df802564..176d23fc 100644 --- a/internal/lockfile/lockfile.go +++ b/internal/lockfile/lockfile.go @@ -171,8 +171,8 @@ func ValidateUpstreamCommit( if !exists { return "", fmt.Errorf( - "no lock file for upstream component %#q", - componentName) + "no lock file for upstream component %#q; run 'azldev component update %s' to create one", + componentName, componentName) } lock, err := Load(fs, lockPath) @@ -182,8 +182,8 @@ func ValidateUpstreamCommit( if lock.UpstreamCommit == "" { return "", fmt.Errorf( - "lock file for %#q has no upstream-commit", - componentName) + "lock file for %#q has no upstream-commit; run 'azldev component update %s' to populate", + componentName, componentName) } if configUpstreamCommit != "" && lock.UpstreamCommit != configUpstreamCommit { @@ -195,7 +195,7 @@ func ValidateUpstreamCommit( return lock.UpstreamCommit, nil } -// ValidateConsistency checks lock files against resolved component configs. +// validateConsistency checks lock files against resolved component configs. // For each upstream component, verifies a lock file exists and any explicit // upstream-commit pin matches. When checkOrphans is true, also detects orphan // lock files (components removed from config). Orphan detection should only @@ -204,7 +204,7 @@ func ValidateUpstreamCommit( // // Returns sorted lists of components with missing/stale locks and orphan // component names. Returns an error if any issues are found. -func ValidateConsistency( +func validateConsistency( fs opctx.FS, lockDir string, components map[string]projectconfig.ComponentConfig, @@ -221,7 +221,7 @@ func ValidateConsistency( if _, validateErr := ValidateUpstreamCommit( fs, lockDir, name, comp.Spec.UpstreamCommit, ); validateErr != nil { - slog.Debug("Lock validation failed", "component", name, "error", validateErr) + slog.Warn("Lock validation failed", "component", name, "error", validateErr) missingOrStale = append(missingOrStale, name) } diff --git a/internal/lockfile/lockfile_test.go b/internal/lockfile/lockfile_test.go index 61979a4c..53d20aa8 100644 --- a/internal/lockfile/lockfile_test.go +++ b/internal/lockfile/lockfile_test.go @@ -458,6 +458,7 @@ func TestValidateUpstreamCommit(t *testing.T) { func TestValidateConsistency(t *testing.T) { t.Run("all valid", func(t *testing.T) { memFS := afero.NewMemMapFs() + store := lockfile.NewStore(memFS, testLockDir) lock := lockfile.New() lock.UpstreamCommit = testCommitHash @@ -468,7 +469,7 @@ func TestValidateConsistency(t *testing.T) { "curl": {Spec: projectconfig.SpecSource{SourceType: projectconfig.SpecSourceTypeUpstream}}, } - stale, orphans, err := lockfile.ValidateConsistency(memFS, testLockDir, components, true) + stale, orphans, err := store.ValidateConsistency(components, true) require.NoError(t, err) assert.Empty(t, stale) assert.Empty(t, orphans) @@ -476,18 +477,20 @@ func TestValidateConsistency(t *testing.T) { t.Run("missing lock", func(t *testing.T) { memFS := afero.NewMemMapFs() + store := lockfile.NewStore(memFS, testLockDir) components := map[string]projectconfig.ComponentConfig{ "curl": {Spec: projectconfig.SpecSource{SourceType: projectconfig.SpecSourceTypeUpstream}}, } - stale, _, err := lockfile.ValidateConsistency(memFS, testLockDir, components, true) + stale, _, err := store.ValidateConsistency(components, true) require.Error(t, err) assert.Contains(t, stale, "curl") }) t.Run("orphan lock file", func(t *testing.T) { memFS := afero.NewMemMapFs() + store := lockfile.NewStore(memFS, testLockDir) lock := lockfile.New() lock.UpstreamCommit = testCommitHash @@ -496,13 +499,14 @@ func TestValidateConsistency(t *testing.T) { components := map[string]projectconfig.ComponentConfig{} - _, orphans, err := lockfile.ValidateConsistency(memFS, testLockDir, components, true) + _, orphans, err := store.ValidateConsistency(components, true) require.Error(t, err) assert.Contains(t, orphans, "removed-pkg") }) t.Run("local component with lock is not orphan", func(t *testing.T) { memFS := afero.NewMemMapFs() + store := lockfile.NewStore(memFS, testLockDir) lock := lockfile.New() lock.InputFingerprint = "sha256:local-fp" @@ -514,19 +518,20 @@ func TestValidateConsistency(t *testing.T) { "curl": {Spec: projectconfig.SpecSource{SourceType: projectconfig.SpecSourceTypeLocal}}, } - _, orphans, err := lockfile.ValidateConsistency(memFS, testLockDir, components, true) + _, orphans, err := store.ValidateConsistency(components, true) require.NoError(t, err) assert.Empty(t, orphans) }) t.Run("local components skipped", func(t *testing.T) { memFS := afero.NewMemMapFs() + store := lockfile.NewStore(memFS, testLockDir) components := map[string]projectconfig.ComponentConfig{ "local-pkg": {Spec: projectconfig.SpecSource{SourceType: projectconfig.SpecSourceTypeLocal}}, } - stale, orphans, err := lockfile.ValidateConsistency(memFS, testLockDir, components, true) + stale, orphans, err := store.ValidateConsistency(components, true) require.NoError(t, err) assert.Empty(t, stale) assert.Empty(t, orphans) @@ -534,6 +539,7 @@ func TestValidateConsistency(t *testing.T) { t.Run("unspecified source type validated like upstream", func(t *testing.T) { memFS := afero.NewMemMapFs() + store := lockfile.NewStore(memFS, testLockDir) // Component with unspecified source type (inherits upstream from defaults) // but no lock file — should be flagged as missing. @@ -541,13 +547,14 @@ func TestValidateConsistency(t *testing.T) { "curl": {}, } - stale, _, err := lockfile.ValidateConsistency(memFS, testLockDir, components, true) + stale, _, err := store.ValidateConsistency(components, true) require.Error(t, err) assert.Contains(t, stale, "curl") }) t.Run("unspecified source type with valid lock passes", func(t *testing.T) { memFS := afero.NewMemMapFs() + store := lockfile.NewStore(memFS, testLockDir) lock := lockfile.New() lock.UpstreamCommit = testCommitHash @@ -558,7 +565,7 @@ func TestValidateConsistency(t *testing.T) { "curl": {}, } - stale, orphans, err := lockfile.ValidateConsistency(memFS, testLockDir, components, true) + stale, orphans, err := store.ValidateConsistency(components, true) require.NoError(t, err) assert.Empty(t, stale) assert.Empty(t, orphans) @@ -566,6 +573,7 @@ func TestValidateConsistency(t *testing.T) { t.Run("mixed stale and orphan", func(t *testing.T) { memFS := afero.NewMemMapFs() + store := lockfile.NewStore(memFS, testLockDir) // Create orphan lock. orphanLock := lockfile.New() @@ -578,7 +586,7 @@ func TestValidateConsistency(t *testing.T) { "missing": {Spec: projectconfig.SpecSource{SourceType: projectconfig.SpecSourceTypeUpstream}}, } - stale, orphans, err := lockfile.ValidateConsistency(memFS, testLockDir, components, true) + stale, orphans, err := store.ValidateConsistency(components, true) require.Error(t, err) assert.Contains(t, stale, "missing") assert.Contains(t, orphans, "orphan") @@ -588,6 +596,7 @@ func TestValidateConsistency(t *testing.T) { func TestPruneOrphans(t *testing.T) { t.Run("prunes removed components", func(t *testing.T) { memFS := afero.NewMemMapFs() + store := lockfile.NewStore(memFS, testLockDir) lock := lockfile.New() lock.UpstreamCommit = testCommitHash @@ -596,8 +605,6 @@ func TestPruneOrphans(t *testing.T) { components := map[string]projectconfig.ComponentConfig{} - store := lockfile.NewStore(memFS, testLockDir) - pruned, err := store.PruneOrphans(components) require.NoError(t, err) assert.Equal(t, 1, pruned) @@ -609,6 +616,7 @@ func TestPruneOrphans(t *testing.T) { t.Run("local component lock preserved", func(t *testing.T) { memFS := afero.NewMemMapFs() + store := lockfile.NewStore(memFS, testLockDir) lock := lockfile.New() lock.InputFingerprint = "sha256:local-fp" @@ -620,8 +628,6 @@ func TestPruneOrphans(t *testing.T) { "curl": {Spec: projectconfig.SpecSource{SourceType: projectconfig.SpecSourceTypeLocal}}, } - store := lockfile.NewStore(memFS, testLockDir) - pruned, err := store.PruneOrphans(components) require.NoError(t, err) assert.Equal(t, 0, pruned, "local component locks should not be pruned") @@ -629,6 +635,7 @@ func TestPruneOrphans(t *testing.T) { t.Run("preserves valid locks", func(t *testing.T) { memFS := afero.NewMemMapFs() + store := lockfile.NewStore(memFS, testLockDir) lock := lockfile.New() lock.UpstreamCommit = testCommitHash @@ -639,8 +646,6 @@ func TestPruneOrphans(t *testing.T) { "curl": {Spec: projectconfig.SpecSource{SourceType: projectconfig.SpecSourceTypeUpstream}}, } - store := lockfile.NewStore(memFS, testLockDir) - pruned, err := store.PruneOrphans(components) require.NoError(t, err) assert.Equal(t, 0, pruned) @@ -652,6 +657,7 @@ func TestPruneOrphans(t *testing.T) { t.Run("unspecified source type is not orphan", func(t *testing.T) { memFS := afero.NewMemMapFs() + store := lockfile.NewStore(memFS, testLockDir) lock := lockfile.New() lock.UpstreamCommit = testCommitHash @@ -663,7 +669,7 @@ func TestPruneOrphans(t *testing.T) { "curl": {}, } - _, orphans, err := lockfile.ValidateConsistency(memFS, testLockDir, components, true) + _, orphans, err := store.ValidateConsistency(components, true) require.NoError(t, err) assert.Empty(t, orphans) }) @@ -674,6 +680,7 @@ func TestPruneOrphans(t *testing.T) { // components would report 199 locks as orphans and fail. func TestValidateConsistency_FilteredSetSkipsOrphans(t *testing.T) { memFS := afero.NewMemMapFs() + store := lockfile.NewStore(memFS, testLockDir) // Create locks for curl and bash. for _, name := range []string{"curl", "bash"} { @@ -688,13 +695,13 @@ func TestValidateConsistency_FilteredSetSkipsOrphans(t *testing.T) { "curl": {Spec: projectconfig.SpecSource{SourceType: projectconfig.SpecSourceTypeUpstream}}, } - stale, orphans, err := lockfile.ValidateConsistency(memFS, testLockDir, filteredComponents, false) + stale, orphans, err := store.ValidateConsistency(filteredComponents, false) require.NoError(t, err) assert.Empty(t, stale) assert.Empty(t, orphans, "orphan detection should be skipped on filtered commands") // Same call with checkOrphans=true WOULD report bash as orphan. - _, orphansAll, errAll := lockfile.ValidateConsistency(memFS, testLockDir, filteredComponents, true) + _, orphansAll, errAll := store.ValidateConsistency(filteredComponents, true) require.Error(t, errAll) assert.Contains(t, orphansAll, "bash") } @@ -777,8 +784,8 @@ func TestStoreGet_CorruptLockReturnsError(t *testing.T) { assert.Equal(t, "fixed", loaded.UpstreamCommit) } -func TestValidateAndSuggestFixes(t *testing.T) { - t.Run("no issues no suggestion", func(t *testing.T) { +func TestStoreValidateConsistency(t *testing.T) { + t.Run("no issues", func(t *testing.T) { memFS := afero.NewMemMapFs() store := lockfile.NewStore(memFS, testLockDir) @@ -791,16 +798,13 @@ func TestValidateAndSuggestFixes(t *testing.T) { "curl": {Spec: projectconfig.SpecSource{SourceType: projectconfig.SpecSourceTypeUpstream}}, } - var suggestions []string - - err := store.ValidateAndSuggestFixes(components, false, func(s string) { - suggestions = append(suggestions, s) - }) + stale, orphans, err := store.ValidateConsistency(components, false) require.NoError(t, err) - assert.Empty(t, suggestions) + assert.Empty(t, stale) + assert.Empty(t, orphans) }) - t.Run("few stale suggests specific update", func(t *testing.T) { + t.Run("stale components returned", func(t *testing.T) { memFS := afero.NewMemMapFs() store := lockfile.NewStore(memFS, testLockDir) @@ -809,18 +813,14 @@ func TestValidateAndSuggestFixes(t *testing.T) { "bash": {Spec: projectconfig.SpecSource{SourceType: projectconfig.SpecSourceTypeUpstream}}, } - var suggestions []string - - err := store.ValidateAndSuggestFixes(components, false, func(s string) { - suggestions = append(suggestions, s) - }) + stale, _, err := store.ValidateConsistency(components, false) require.Error(t, err) - require.Len(t, suggestions, 1) - assert.Contains(t, suggestions[0], "azldev component update") - assert.NotContains(t, suggestions[0], "-a") + assert.Len(t, stale, 2) + assert.Contains(t, stale, "curl") + assert.Contains(t, stale, "bash") }) - t.Run("orphans suggest update all", func(t *testing.T) { + t.Run("orphans returned with checkOrphans", func(t *testing.T) { memFS := afero.NewMemMapFs() store := lockfile.NewStore(memFS, testLockDir) @@ -831,13 +831,8 @@ func TestValidateAndSuggestFixes(t *testing.T) { components := map[string]projectconfig.ComponentConfig{} - var suggestions []string - - err := store.ValidateAndSuggestFixes(components, true, func(s string) { - suggestions = append(suggestions, s) - }) + _, orphans, err := store.ValidateConsistency(components, true) require.Error(t, err) - require.Len(t, suggestions, 1) - assert.Contains(t, suggestions[0], "update -a") + assert.Contains(t, orphans, "removed") }) } diff --git a/internal/lockfile/store.go b/internal/lockfile/store.go index a5685fe1..5bdcaeaf 100644 --- a/internal/lockfile/store.go +++ b/internal/lockfile/store.go @@ -7,7 +7,6 @@ import ( "errors" "fmt" "log/slog" - "strings" "sync" "github.com/microsoft/azure-linux-dev-tools/internal/global/opctx" @@ -16,13 +15,20 @@ import ( // LockReader provides read-only access to per-component lock files. Use this // interface for commands that consume lock state but should not modify it -// (e.g., render, build). +// (e.g., render, build, validation). type LockReader interface { // Get returns the lock for a component. Returns an error if the lock file // does not exist or cannot be parsed. Get(componentName string) (*ComponentLock, error) // Exists checks whether a lock file exists for the given component. Exists(componentName string) (bool, error) + // ValidateConsistency checks lock files against the resolved component + // configs. Returns sorted lists of components with missing/stale locks + // and orphan component names. + ValidateConsistency( + components map[string]projectconfig.ComponentConfig, + checkOrphans bool, + ) (missingOrStale, orphans []string, err error) } // LockWriter extends [LockReader] with write operations. Use this interface @@ -198,7 +204,7 @@ func (s *Store) ValidateConsistency( components map[string]projectconfig.ComponentConfig, checkOrphans bool, ) (missingOrStale, orphans []string, err error) { - return ValidateConsistency(s.fs, s.lockDir, components, checkOrphans) + return validateConsistency(s.fs, s.lockDir, components, checkOrphans) } // FindOrphanLockFiles returns component names that have lock files but no @@ -248,29 +254,3 @@ func (s *Store) PruneOrphans( return pruned, nil } - -// ValidateAndSuggestFixes checks consistency and formats fix suggestions. -// Returns an error if validation fails, with human-readable fix suggestions -// appended via the addSuggestion callback. -func (s *Store) ValidateAndSuggestFixes( - components map[string]projectconfig.ComponentConfig, - checkOrphans bool, - addSuggestion func(string), -) error { - const maxIssuesForDetailedSuggestion = 10 - - stale, orphans, validateErr := s.ValidateConsistency(components, checkOrphans) - if validateErr == nil { - return nil - } - - if len(orphans) > 0 || len(stale) > maxIssuesForDetailedSuggestion { - addSuggestion("run 'azldev component update -a' to fix all lock file issues") - } else if len(stale) > 0 { - addSuggestion(fmt.Sprintf( - "run 'azldev component update %s'", - strings.Join(stale, " "))) - } - - return validateErr -} diff --git a/internal/projectconfig/configfile.go b/internal/projectconfig/configfile.go index e4bb4806..cdd62014 100644 --- a/internal/projectconfig/configfile.go +++ b/internal/projectconfig/configfile.go @@ -6,6 +6,7 @@ package projectconfig import ( "fmt" "net/url" + "os" "github.com/go-playground/validator/v10" "github.com/microsoft/azure-linux-dev-tools/internal/global/opctx" @@ -88,6 +89,21 @@ func (f ConfigFile) Validate() error { } } + // Per-component snapshot timestamps are not allowed when lock validation is + // enabled. Components inherit the snapshot from the distro/group + // default-component-config or the project's default-distro. Per-component + // snapshots would create non-deterministic builds that the lock file cannot + // reliably track. Use an explicit 'upstream-commit' pin instead. + // + // Gated behind AZLDEV_ENABLE_LOCK_VALIDATION during rollout. + // NOTE: Once the env var gate is removed, snapshot rejection becomes + // unconditional — per-component snapshots are permanently disallowed. + // Do NOT move this into [ComponentFilter.SkipLockValidation]: the snapshot check + // must not be disableable post-rollout. + //nolint:godox // tracked by TODO(lockfiles) tag. + // TODO(lockfiles): remove env var gate; snapshot rejection becomes unconditional. + lockValidationEnabled := os.Getenv("AZLDEV_ENABLE_LOCK_VALIDATION") == "1" + // Validate overlay configurations for each component. for componentName, component := range f.Components { for i, overlay := range component.Overlays { @@ -106,6 +122,14 @@ func (f ConfigFile) Validate() error { if err := validateSourceFiles(component.SourceFiles, componentName); err != nil { return err } + + if lockValidationEnabled && component.Spec.UpstreamDistro.Snapshot != "" { + return fmt.Errorf( + "component %#q has a per-component 'snapshot' on 'upstream-distro'; "+ + "snapshots should be set on the distro or group default-component-config, "+ + "or use 'upstream-commit' to pin a specific commit", + componentName) + } } return nil diff --git a/internal/projectconfig/configfile_test.go b/internal/projectconfig/configfile_test.go index 6492f13e..d7264d55 100644 --- a/internal/projectconfig/configfile_test.go +++ b/internal/projectconfig/configfile_test.go @@ -299,3 +299,24 @@ func TestProjectConfigFileValidation_UnsupportedOriginType(t *testing.T) { assert.Contains(t, err.Error(), "unsupported 'origin' type") assert.Contains(t, err.Error(), "ftp") } + +func TestProjectConfigFileValidation_PerComponentSnapshotDisallowed(t *testing.T) { + t.Setenv("AZLDEV_ENABLE_LOCK_VALIDATION", "1") + + file := projectconfig.ConfigFile{ + Components: map[string]projectconfig.ComponentConfig{ + "test-component": { + Spec: projectconfig.SpecSource{ + SourceType: projectconfig.SpecSourceTypeUpstream, + UpstreamDistro: projectconfig.DistroReference{ + Snapshot: "2026-01-01T00:00:00Z", + }, + }, + }, + }, + } + err := file.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "snapshot") + assert.Contains(t, err.Error(), "test-component") +} diff --git a/internal/projectconfig/distro.go b/internal/projectconfig/distro.go index 89fe7e22..cd0aa85a 100644 --- a/internal/projectconfig/distro.go +++ b/internal/projectconfig/distro.go @@ -18,7 +18,9 @@ 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" fingerprint:"-"` + // Note: set this on the distro or group default-component-config, not on individual components. + // Per-component snapshots are rejected when lock validation is enabled. + Snapshot string `toml:"snapshot,omitempty" json:"snapshot,omitempty" jsonschema:"format=date-time,title=Snapshot,description=Snapshot timestamp for source code. Set on the distro or group default-component-config only — per-component snapshots are not allowed." fingerprint:"-"` } // Implements the [Stringer] interface for [DistroReference]. diff --git a/scenario/__snapshots__/TestMCPServerMode_1.snap.json b/scenario/__snapshots__/TestMCPServerMode_1.snap.json index bd0bd5ea..cec130da 100755 --- a/scenario/__snapshots__/TestMCPServerMode_1.snap.json +++ b/scenario/__snapshots__/TestMCPServerMode_1.snap.json @@ -78,6 +78,11 @@ "description": "only enable minimal output", "type": "boolean" }, + "skip-lock-validation": { + "default": true, + "description": "skip lock file consistency checks", + "type": "boolean" + }, "spec-path": { "description": "Spec path", "type": "string" @@ -166,6 +171,11 @@ "description": "only enable minimal output", "type": "boolean" }, + "skip-lock-validation": { + "default": true, + "description": "skip lock file consistency checks", + "type": "boolean" + }, "spec-path": { "description": "Spec path", "type": "string" diff --git a/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap b/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap index d90a8344..d7e950d7 100755 --- a/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap +++ b/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap @@ -425,7 +425,7 @@ "type": "string", "format": "date-time", "title": "Snapshot", - "description": "If specified use source code as it existed at this date/time" + "description": "Snapshot timestamp for source code. Set on the distro or group default-component-config only — per-component snapshots are not allowed." } }, "additionalProperties": false, diff --git a/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap b/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap index d90a8344..d7e950d7 100755 --- a/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap +++ b/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap @@ -425,7 +425,7 @@ "type": "string", "format": "date-time", "title": "Snapshot", - "description": "If specified use source code as it existed at this date/time" + "description": "Snapshot timestamp for source code. Set on the distro or group default-component-config only — per-component snapshots are not allowed." } }, "additionalProperties": false, diff --git a/schemas/azldev.schema.json b/schemas/azldev.schema.json index d90a8344..d7e950d7 100644 --- a/schemas/azldev.schema.json +++ b/schemas/azldev.schema.json @@ -425,7 +425,7 @@ "type": "string", "format": "date-time", "title": "Snapshot", - "description": "If specified use source code as it existed at this date/time" + "description": "Snapshot timestamp for source code. Set on the distro or group default-component-config only — per-component snapshots are not allowed." } }, "additionalProperties": false,