Refactor and Auto Forward Ports#277
Draft
nicholasSUSE wants to merge 26 commits intorancher:masterfrom
Draft
Conversation
- replace path cosntants from config package - update filesystem.WalkDir calls to include Soft Error mode from config package.
Introduce pkg/config as a centralized configuration management package following a "load once, use everywhere" pattern. This package consolidates previously scattered configuration logic into a single source of truth. Core components: config.go: - Config struct: Central configuration holder with repository context, git interface, version rules, tracked charts, and assets map - Init(): Main entry point for loading and validating all configuration - Validates git repository state, file structure, and loads all config files - Integrates with existing pkg/filesystem, pkg/git, and pkg/options versions.go: - VersionRules: Manages version retention policies and branch versioning - Loads config/versionRules.yaml with branch-version mapping - Calculates MinMajor based on minor-retention-policy - Provides version validation helpers (IsVersionCandidate, IsVersionForCurrentLine) charts.go: - TrackedCharts: Manages active, legacy, and deprecated chart tracking - Loads config/trackCharts.yaml with chart status and targets - GetChartByName(): Intelligently handles chart names and target suffixes (e.g., "fleet" and "fleet-crd" both resolve to parent chart) - IsActive/IsActiveOrLegacy: Status check helpers path.go: - Centralized path constants for release operations (PathReleaseYaml, PathIndexYaml, PathAssetsDir, etc.) - Chart operation paths (PathPackagesDir, PathChangesDir, PathOverlayDir, PathPatchDir, PathExcludeDir) - Replaces scattered path definitions across the codebase context.go: - Context integration for accessing Config throughout the application - WithConfig/FromContext: Standard context pattern for config access - SetSoftError/IsSoftError: Runtime error handling mode control Design decisions: - Trade-off: Commands may load more data than strictly needed, but performance impact is negligible (YAML parsing + simple calculations) - Benefit: Simplified code, single source of truth, easier testing - Git repository must be clean before operations (prevents data loss)
- lifecycle pkg: parts were already moved to the new `config` pkg; other parts are migrated in future commits. - util and other pkgs were all moved inside config centralizing repository setup.
Migrate and simplify lifecycle validation logic from pkg/lifecycle (status.go, version_rules.go) into pkg/validate (lifecycle.go, versions.go), eliminating complex state tracking and git operations. FROM pkg/lifecycle (upstream - 9 files): status.go: - Complex Status struct with 10+ fields tracking detailed asset states - Tight coupling with Dependencies struct - Git cloning and branch switching for asset comparison - Multi-file logging (3 separate log files) version_rules.go: - Complex Version struct with Min/Max boundaries - Manual version range calculations (getMinMaxVersionInts) - Manual JSON loading and validation TO pkg/validate (refactored - 2 files): lifecycle.go: - Simplified Status struct: ToRelease, ToForwardPort (2 fields only) - Uses helm.RemoteIndexYaml() instead of git operations - 3-stage pipeline: fetch → filter deprecated → categorize - Single YAML state file output versions.go: - FilterPortsAndToRelease() for clean categorization - VersionPrefixMatch() for version validation - All functions use config.VersionRules from context Key improvements: 1. Eliminated git workflow (no cloning, no branch switching) 2. Removed complex state tracking (10+ fields → 2 fields) 3. Removed multi-file logging (3 files → 1 YAML state file) 4. Removed Dependencies pattern (use config.FromContext) 5. Simplified version logic (direct prefix matching) Result: 9 files → 2 files, ~70% less code, faster execution, easier to test and maintain
…port Migrate helm package to use centralized config package and add functionality to fetch remote index.yaml files from GitHub branches. Config integration: - Replace pkg/path constants with config.Path* equivalents - Use config.FromContext(ctx) for accessing configuration - Remove rootFs parameter from CreateOrUpdateHelmIndex (use cfg.RootFS) - Update CheckVersionStandards to use cfg.VersionRules.AllowedCandidates/AllowedMetadata New functions for lifecycle validation: - RemoteIndexYaml(): Fetch index.yaml from GitHub branches via HTTP * Eliminates need for git cloning in lifecycle validation * 10-second timeout for reliability * Used by pkg/validate/lifecycle.go to compare dev vs release branches - GetAssetsVersionsMap(): Load and convert local index.yaml to simple map - ConvertIndexToVersionsMap(): Pure converter function (IndexFile → map[string][]string) - LoadChartYaml(): Load Chart.yaml metadata for specific chart/version Improvements: - CheckVersionStandards now validates against dynamic config rules - Better separation between IndexFile operations and simple map conversions - Added comprehensive documentation for all new functions - Fixed typo: "standars" → "standards" This enables pkg/validate/lifecycle.go to fetch remote indexes without git operations, significantly improving performance and reducing complexity.
Make YAML helper functions more flexible and accessible to support the config and validate package refactoring. yaml.go changes: - Export SafeDecodeYaml (was safeDecodeYaml) * Enables other packages to use safe YAML decoding with fallback * Supports decoding from StreamReader callbacks - Generalize UpdateYamlFile signature (map[string][]string → any) * Now accepts any data structure for YAML encoding * Used by pkg/validate/lifecycle.go to write Status struct assets.go changes: - Update DecodeValueYamlInTgz to use exported SafeDecodeYaml These changes enable pkg/validate/lifecycle.go to write state files and improve overall YAML handling consistency across the codebase
Simplify ChartsRepository validation by using centralized config instead of passing multiple parameters through function signatures. Function signature changes: - ChartsRepository: 9 parameters → 5 parameters * Removed: cli.Context, repoRoot, rootFs, csOptions * Access via: config.FromContext(ctx) - isGitClean: removed repoRoot parameter (use cfg.Root) - generateChartsConcurrently: removed repoRoot, csOptions, rootFs parameters Config integration: - Use cfg.Root instead of repoRoot parameter - Use cfg.RootFS instead of rootFs parameter - Use cfg.ChartsScriptOptions instead of csOptions parameter - All configuration accessed via config.FromContext(ctx) Package reorganization: - ValidateIcons moved from pkg/auto to pkg/validate - ArchiveCharts moved from pkg/zip to pkg/helm - Updated helm.CreateOrUpdateHelmIndex calls (removed rootFs parameter) Improvements: - Added input parameter logging for debugging - Added validation: prevent both localMode and remoteMode flags - Cleaner function signatures with context-based config access - Consistent with config package architecture pattern This continues the refactoring to centralize configuration management and reduce parameter passing throughout the codebase.
Relocate icon validation logic to pkg/validate package and integrate with centralized config architecture. Changes: - Move ValidateIcons function to pkg/validate package - Update function signature to use config.FromContext(ctx) * Removed: rootFs parameter * Access via: cfg.RootFS from context - Add IsIconException helper function * Checks for -crd suffix (always skip icon validation) * Uses cfg.TrackedCharts.GetChartByName() for chart lookup * Checks trackedChart.Icon field to determine if icon is required - Add loadAndCheckIconPrefix helper function * Validates icon has file:// prefix * Verifies icon file exists in filesystem Config integration: - Use config.PathReleaseYaml instead of hardcoded string - Use config.PathTrackChartsYaml for error messages - Load release options via cfg.RootFS - Access tracked charts via cfg.TrackedCharts This consolidates validation logic in pkg/validate and follows the pattern of using centralized configuration via context instead of parameter passing.
Update Package methods to use centralized config and remove deprecated icon download functionality. GenerateCharts changes: - Remove omitBuildMetadataOnExport parameter - Get value from cfg.ChartsScriptOptions.OmitBuildMetadataOnExport - Update helm.CreateOrUpdateHelmIndex call (removed rootFs parameter) - Use config.FromContext(ctx) for configuration access Removed functionality: - DownloadIcon method (60 lines) - icon handling moved elsewhere - Unused imports: pkg/icons, pkg/path, helmLoader, chartutil, strings This completes the config package integration for chart generation and aligns with the validate/charts.go refactoring.
…cle dependency Migrate validate.go to use centralized config package and remove the complex lifecycle dependency initialization from asset comparison logic. Config integration: - Replace all path.* constants with config.Path* equivalents: * PathReleaseYaml, PathReleasedAssetsDir, PathAssetsDir - Add config.IsSoftError(ctx) support for filesystem.CompareDirs Removed lifecycle dependency: - Remove lifecycle.InitDependencies initialization (7 lines) - Remove lifecycle-based version validation in CompareGeneratedAssets - Simplify: all untracked charts are now reported (no version filtering) - This decouples asset validation from lifecycle version rules Package reorganization: - standardize.RestructureChartsAndAssets → helm.RestructureChartsAndAssets - zip.DumpAssets → helm.DumpAssets - Consolidates Helm-related operations in pkg/helm Code cleanup: - Remove unused range variable in validateExceptions This simplifies the upstream validation flow by removing the dependency on lifecycle.InitDependencies and its complex version checking logic. The validation now focuses purely on comparing local vs upstream assets without version-based filtering.
Improve RC validation to check both container images and chart versions, and integrate with config package architecture. Function signature changes: - Remove repoRoot parameter (use config.FromContext) - Change return type: map[string][]string → error - Function now validates and fails instead of just collecting data Enhanced validation logic: - Check RC tags in container images (existing) - Check RC versions in chart metadata (NEW via charts.CheckRCCharts) - Fail immediately if ANY RC found in either images or charts - Detailed error logging showing both rcImageTagMap and rcChartVersionMap Config integration: - Use cfg.Root instead of repoRoot parameter - Add config and charts package imports This provides complete RC validation coverage, ensuring neither images nor charts contain release candidate versions before production release.
Complete rewrite of forward-port logic from complex struct-based orchestration to simple function-based direct asset copying. Removed old implementation: - ForwardPort struct with 5 fields and 4 methods - Complex PR organization and multi-branch git operations - yq command dependency - Fork remote URL validation New implementation: - Single ForwardPort(ctx, sourceBranch) function - Uses helm.RemoteIndexYaml() instead of git clone - Integrated config.TrackedCharts filtering - Immediate commits per chart version - RC version detection via config.VersionRules Result: 158 lines → 149 lines, simpler flow, faster execution.
Replace lifecycle.InitDependencies with config.Config across chart_bump.go and versioning.go, completing the lifecycle package removal from pkg/auto. Key changes: - Bump struct: drop releaseYaml, versionRules, repo, rootFs fields; simplify assetsVersionsMap from []lifecycle.Asset to []string; move bumpVersion and branchLine into the target sub-struct - BumpChart promoted from receiver method to standalone function with upfront input validation; SetupBump made private (setupBump) - All Bump methods now receive *config.Config explicitly; git and filesystem operations use cfg.Repo and cfg.RootFS instead of struct-held references - applyVersionRules: tighten branch-line transition logic — only accept major diff == 1 as a forward-port case; error on diff > 1 or backwards regression - getLatestVersionRecursively: add empty-slice guard and extend pre-release filter to cover -alpha and -beta in addition to -rc - Expand test coverage: add Test_loadVersions and Test_applyVersionRules; update Test_calculateNextVersion and Test_listAssetsToForwardPort to use config.Config instead of lifecycle types
Replace lifecycle.Dependencies/Status with config.Config and validate.Status across release.go, completing the lifecycle package removal from pkg/auto. Key changes: - Release struct renamed to Asset (pure value object: Tgz, Path, Version, Chart); drop git, VR, ReleaseYamlPath, ForkRemoteURL fields - InitRelease eliminated; replaced by standalone Release() orchestration function that loads config and status internally via config.FromContext and validate.LoadStateFile - loadAssetInfo: drop cfg parameter (unused); status fields renamed from AssetsToBeReleased/AssetsToBeForwardPorted to ToRelease/ToForwardPort; []lifecycle.Asset simplified to []string; release.yaml existence pre-check removed - PullAsset, UpdateReleaseYaml, PullIcon promoted from receiver methods to package-level functions with explicit parameters - loadChartYaml private function deleted; replaced by helm.LoadChartYaml, removing direct helm.sh/v3 SDK imports from this file - All error paths in Release() unified to return err (previously mixed logger.Fatal) - Add Test_loadAssetInfo (5 cases) covering ToRelease hit, ToForwardPort fallback, chart not found, and version not found paths
Simplify main.go by moving all initialization and orchestration logic into
the respective packages, completing the lifecycle package removal.
Key changes:
- init(): initialize global Ctx via config.Init + config.WithConfig, moving
getRepoRoot() logic here; eliminates per-handler context.Background() +
getRepoRoot() pattern across all command functions
- Drop parseScriptOptions(), getRepoRoot(), getPackages() helpers; their
logic is now owned by config and the respective packages
- Remove lifecycle.InitDependencies and lifecycle.LoadState from all handlers;
replaced by single-call package functions (auto.ForwardPort, auto.Release,
auto.BumpChart, validate.LifecycleStatus, validate.PullRequestCheckPoints,
validate.CompareIndexFiles)
- Remove zip, standardize, update, options, path, util, lifecycle imports;
zip.ArchiveCharts/DumpAssets moved to helm pkg, standardize.Restructure to
helm pkg, update.ApplyUpstreamTemplate to puller pkg
- Drop DockerUser/Password, StagingUser/Password, PrimeUser/Password globals
and their env var constants; syncRegistries simplified to (PrimeURL, CustomOCIPath)
- Add BranchVersion global; branchVersionFlag gains Destination binding;
chart-bump uses BranchVersion instead of Branch, auto-forward-port drops
forkFlag (validation moved inside auto.ForwardPort)
- lifecycle-status drops chartFlag (chart filtering moved inside validate.LifecycleStatus)
- Fix: auto.Release(Ctx, ChartVersion, CurrentChart) — was swapped
- Globals and constants reorganized into semantic groups (Configuration,
Chart Owners, Release Team, Registries, Branch, Validation)
…state DEPENDENCY_MAP.md: full rewrite to reflect current package structure. - Remove pkg/lifecycle, pkg/standardize, pkg/zip, pkg/update, pkg/path, pkg/util from layer diagram and all dependency matrices - Update all 12 package import matrices to verified current imports - Add "Removed Packages" table mapping each deleted package to its destination - Update config.Init() signature (5 params → 3), add Forward Port and Lifecycle Status chains to Critical Dependency Chains section REFACTORING_SUMMARY.md: correct inaccuracies introduced during refactor. - Fix branch name: fix-rc-ordering-with-alphas-betas → refactor-auto-forward-ports - Add pkg/path, pkg/util, pkg/update to Deleted Files; add corresponding new files: config/path.go, puller/update.go - Replace cosign.go with slsactl.go in AFTER structure and New Files - Add config/path.go to AFTER structure tree - Remove stale pkg/path/path.go and pkg/update/pull.go from Changed Files - Update Package Responsibilities table: drop path row, extend descriptions to reflect absorbed functionality (zip, standardize, lifecycle, slsa, cache) - Fix "no global state (except util/logger)" → "except logger" - Add path, util, update entries to Migration Checklist Delete CODEBASE_ANALYSIS.md: pre-refactor snapshot superseded by the updated DEPENDENCY_MAP.md and REFACTORING_SUMMARY.md.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues: