Ntlaletsi70 patch 1 - #297
Open
ntlaletsi70 wants to merge 8 commits into
Open
Conversation
GitHub-hosted runner minutes are no longer viable to pay for; route every job across all workflows to the self-hosted zenith-runners-pool instead. The SLSA provenance reusable-workflow job is unaffected — it controls its own runner internally for the trusted-builder attestation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
) Pins the runner image via .actrc and documents which secrets each workflow needs via .secrets.example, so CI steps can be exercised locally before pushing. Also gitignores the real .secrets/.vars/.input files act reads them from, plus two generated report files (beauty-report.html, coverage-dashboard.html) that were leaking into git status. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…291) Resolve{GitRepository,Route,Domain} decode attacker-controlled Contract.Raw JSON from CRs and manually walk nested maps/type assertions beyond what encoding/json validates. Fuzzing the existing xWithContract(raw) test helpers exercises that parsing directly, satisfying the OpenSSF Scorecard Fuzzing check via Go's native testing.F support (go.dev/doc/security/fuzz). Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* ci: move SLSA provenance to its own standalone workflow (#272) Reverts the release-assets.yml integration in favor of a dedicated slsa-provenance.yml matching the upstream generator's own build+ provenance shape. Scopes provenance to the one artifact this repo actually builds (the gomarkdoc docs bundle) rather than also attesting install/CRDs/CLI assets that are just re-bundled from environments-install and environments-cli's own releases — those repos should generate their own provenance for their own output. The build job rebuilds the docs bundle itself so the hash it feeds to the provenance generator is guaranteed to match what it built, rather than assuming byte-identical reproducibility against release-assets.yml's separate docs job. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): match code-docs.yml GPG committer identity to the signing key (#275) Every push to main has been failing at "Import GPG key" with: Committer email "github-actions[bot]@users.noreply.github.com" does not match GPG private key email "actions@github.com" code-docs.yml was the only workflow using a different committer identity (github-actions[bot] / users.noreply.github.com) than the one the shared GPG_PRIVATE_KEY is actually issued for. release.yml and finalize-release.yml already use github-actions / actions@github.com and sign correctly — code-docs.yml now matches. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): heal stale vendor-snapshot cache and simplify coverage summary (#276) * fix(ci): heal stale vendor-snapshot cache and simplify coverage summary Vendor-snapshot fix (validate, coverage in ci.yml; govulncheck in security.yml): go.sum only changes when a required module's version changes. A test newly importing an already-required module's not-yet-vendored subpackage (e.g. pkg/secrets/git/build/build_test.go importing sigs.k8s.io/controller-runtime/pkg/client/fake) leaves go.sum untouched, so the sha256(go.sum) cache tag never busts — the cached snapshot built before that import existed keeps getting restored as current forever. Confirmed via the failed develop CI run (#270, 29609870897): "Restore vendor from snapshot" succeeds, then go test -mod=vendor fails with "cannot find module providing package .../client/fake: import lookup disabled by -mod=vendor". Each of the three jobs now verifies the restored vendor with a plain go build -mod=vendor right after restoring, and if that fails, rebuilds vendor locally and pushes a corrected snapshot under the same tag + :latest before continuing — turning a silent, permanent staleness bug into a self-healing one-time rebuild. Coverage summary (ci.yml coverage job): go tool cover -func emits one row per function, not per package — hundreds of rows in a repo this size. Rewritten to aggregate coverage.out's statement counts by package directory instead, producing one row per package, sorted weakest-first, and collapsed into a <details> block so the step summary stays short by default. Also fixed the module-prefix strip, which referenced a stale github.com/BlanketOps/blanketops-environments/ path that doesn't match this module's actual github.com/blanketops/environments. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(ci): generate a gocov-html coverage report alongside coverage.html Adds beauty-report.html (gocov convert + gocov-html) as a richer, styled coverage report next to the existing plain go tool cover -html output, uploaded in the same coverage-report-<sha> artifact. gocov's tagged releases and current master (last touched 2024-10-11) pin golang.org/x/tools@v0.13.0, which fails to compile under this repo's Go toolchain — confirmed directly (go install fails identically on both the latest tag and master; a plain golang.org/x/tools@v0.13.0 build fails the same way outside of gocov entirely, while @latest builds fine). This is stale upstream, not fixable by tracking a newer gocov version, so the only working path is building gocov from source with a forced newer x/tools via `go mod edit -replace` — verified end-to-end against this repo's real coverage.out before wiring it in. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): staleness probe must include go vet, not just go build Caught live on the first real run of the previous commit: the probe used `go build -mod=vendor ./...`, but build doesn't type-check _test.go files — and the exact missing package this whole fix exists for (sigs.k8s.io/controller-runtime/pkg/client/fake) is only ever imported from a test file. Build reported success, the rebuild/heal steps got skipped as a no-op, and Vet (which does check test files) then failed on the same stale vendor right after — same bug, just moved one step later instead of actually being caught. All three probes (validate, coverage in ci.yml; govulncheck in security.yml) now run go vet -mod=vendor alongside go build before deciding whether to rebuild. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): reset GOPROXY before the vendor-heal fallback's go mod vendor Live failure on the first real govulncheck run of this fix: the rebuild step's go mod vendor hit honnef.co/go/tools/cmd/staticcheck: unrecognized import path "honnef.co/go/tools": https fetch: ... TLS handshake timeout validate's equivalent run (same commit) succeeded, so this may have been one-off network flakiness rather than guaranteed — but the GOPROXY=direct these three jobs set is real and relevant either way: it forces every module, including vanity-domain ones like honnef.co/go/tools, through direct fetch instead of the module proxy. vendor-snapshot (ci.yml) — the job actually designed to run a full `go mod vendor` — never sets GOPROXY at all, relying on the default proxy-first behavior. The heal steps in validate/coverage/govulncheck now reset GOPROXY to that same default immediately before go mod vendor, matching the job that's actually proven to do this reliably. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * fix(deployment): untangle execution + domain layers, fix strategy bug (#281) * fix(deployment): untangle execution + domain layers, fix strategy bug Deployment carried three separate, unreconciled attempts at "dispatch a Provider from an Intent" plus two unreconciled domain result/error models. Audited every file (full diffs or repo-wide grep confirming zero external references) before deleting anything — nothing here had existing test coverage to break. Real bug fixed: K8SProvider.Execute (pkg/apis/deployment/api/kubernetes.go) had a parameter named `intent` shadowing the `intent` package, so `switch intent.Strategy { case intent.Strategy: ... }` compared the value to itself — the first case always matched and executeBlueGreen was unreachable. Every Kubernetes deployment silently executed as Rolling regardless of configured Strategy. Renamed the parameter to dIntent and fixed the case labels to reference the real package constants; added a table-driven test proving the two branches are now actually distinguishable. Deleted as confirmed dead code: - pkg/apis/deployment/api/application/ (5 files) — the oldest BackendSelector.ForIntent design: panics instead of returning errors, no GitOps awareness beyond a hardcoded Flux special-case, and its intent_builder/mapper/status siblings were missing ManifestsRepo/ReconciliationStrategy wiring, a validation check, and RetryOnConflict that the surviving application/ package has. - pkg/apis/deployment/application/backend_selector.go + gitops_decorator.go — a second, later redesign (ProviderRegistry + Provider.Supports() + GitOpsDecorator) that was never wired to DeploymentService, and whose one piece of real logic (GitOpsDecorator.Execute) was a single comment with no actual implementation — while the GitOps path actually in use (KustomizeStrategyProvider.ReconcileKustomization) is fully real. - pkg/apis/deployment/domain/result.go, domain/errors.go, and the Result-consuming half of domain/state.go (DeploymentState, ServiceUnitState, StateFromResult, ServiceUnitStateFromResult) — a third, richer domain model with zero references anywhere outside these three files; the execution path uses model.go's simpler DeploymentResult/DeploymentPhase/ServiceUnitPhase instead. Also promotes Intent to match the per-CR floor layout every other piece of this codebase uses (pkg/apis/packages/intent/ already does this): moved pkg/intent/deployment/ (9 files) to pkg/apis/deployment/intent/, renaming `package deployment` to `package intent` to match — this also drops the explicit `intent` import alias every caller previously needed to avoid colliding with the package's old name. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(deployment): relocate intent to top-level intent/deployment/ Amends this branch's earlier move — pkg/apis/deployment/intent/ was still nested under deployment's own tree, which doesn't match the actual target: Intent as a real 4th architectural pillar, sibling to resolution/, cache/, and core/, not tucked inside pkg/apis/. Moved intent/deployment/ to the repo root and renamed package intent back to package deployment (matching cache/deployment's convention: package name = directory's own name). Every caller re-adds the explicit `intent` import alias to keep every existing call site (intent.DeploymentIntent, intent.StrategyRolling, etc.) unchanged. pkg/apis/packages/intent/ (nested) is now the odd one out against this new top-level convention — flagged as a likely follow-up, not done here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(deployment,packages): relocate intent under pkg/, not top-level Reverses the previous top-level intent/ placement — pkg/apis/ is where all the per-CR domain code already lives, so Intent belongs alongside it under pkg/, not out at the repo root next to resolution/cache/core. - pkg/intent/deployment/ — back to its original location (this is where it started before any of today's moves), package deployment kept as-is from the last relocation. Every caller keeps its explicit `intent` import alias. - pkg/apis/packages/intent/ -> pkg/intent/package/ — the other CR with its own intent subpackage, now matching the same pkg/intent/ convention. Kept `package intent` (directory is named "package", which can't be a Go package name — it's a reserved word), so the 5 existing importers already resolve to the `intent` identifier with no alias needed, same as before the move. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(serviceunit,deployment): give ServiceUnit its own intent layer Neo: "serviceunit must cater for its own resolution... I have the intent resolving serviceunit, not correct, it must resolve itself." Same principle as the earlier resolution/domain/application work for ServiceUnit, now extended to the intent layer: pkg/intent/deployment owned ServiceUnitIntent, RouteIntent, and WorkloadIntent, and built them via ResolveServiceUnitIntent — all logic that's actually about ServiceUnit, not Deployment. New pkg/intent/serviceunit/ (matching the pkg/intent/<cr> convention already established for deployment and package): - ServiceUnitIntent + ResolveServiceUnitIntent (moved from pkg/intent/deployment/serviceunit.go) - RouteIntent (moved from route.go) and WorkloadIntent (moved from workload.go) — both were only ever used by ServiceUnitIntent, so they move with it; zero circular-dependency risk confirmed before moving. - ErrBuildNotReady/ErrInvalidServiceUnit (moved from errors.go) — the two ServiceUnit-specific errors; ErrServiceUnitNotFound and ErrInvalidDeployment stay in deployment's errors.go since they're genuinely Deployment's own. pkg/intent/deployment.DeploymentIntent.ServiceUnits is now []serviceunitIntent.ServiceUnitIntent; every caller across resolve.go, pkg/apis/deployment/application/intent_builder.go, pkg/apis/deployment/render/builders/builder.go, pkg/apis/deployment/api/{kubernetes,ecs}.go updated accordingly. Also relocates IntentBuilder (pkg/apis/deployment/application -> pkg/intent/deployment/intent_builder.go) per a follow-up request — the intent-building logic belongs in the intent package itself, not the application layer. Note for a later pass: this package now has two overlapping ways to build a DeploymentIntent — ResolveDeploymentIntent (resolve.go) and IntentBuilder.Build (intent_builder.go) — the latter is more complete (sets Runtime/Strategy/ReconciliationStrategy/ ManifestsRepo, which the former doesn't), not reconciled here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * test: add coverage for pkg/apis/serviceunit and the cache/* layer (#280) * test: add coverage for pkg/apis/serviceunit and the cache/* layer Both pkg/apis/serviceunit/{domain,application} and every cache/* per-CR package (build, deployment, domain, environment, githubevent, gitrepository, packages, route, serviceunit) plus the shared cache.ObjectCache/NewExternal primitives were at 0% coverage. Total repo coverage: 35.9% -> 48.3%. Adds cache/internal/testutil.FakeExternalCache — an in-memory, JSON-serializing core/cache.ExternalCache fake shared across cache/* tests, mirroring pkg/secrets/internal/testutil's existing pattern. Real bug found and fixed while writing the serviceunit cache tests: cache/serviceunit/serviceunit.go's PublishResolved switched on r.Spec.Type.String(), comparing against literal "static"/"build" — but the generated proto String() returns the full constant name (e.g. "SERVICE_UNIT_TYPE_STATIC"), so neither case ever matched and the image/buildRef field was silently never cached for any ServiceUnit. Fixed to switch on the enum value directly, matching the pattern already used in resolution/serviceunit/resolve.go. Not covered here: cache/adapter's Redis/Memcached backends need a live connection to test meaningfully (and per setup.go's own TODOs, neither is actually wired up yet — NewExternal always returns NoopExternalCache regardless of the configured backend). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(serviceunit): wire ServiceUnitCache into Reconcile Closes the gap flagged after the cache/* coverage pass: every per-CR cache was fully built and tested but never called from any CR's application/Reconcile code. Wires ServiceUnit end to end as the first pattern to prove the integration point, before deciding whether to repeat it for the other 8 CRs. Reconcile now calls cache.PublishResolved after the status write, using the CR's own namespace/name/generation. Best-effort per the cache layer's existing contract (see PublishResolved's doc comment) — its error is discarded, never fails Reconcile. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * docs: regenerate code documentation (#284) docs/code/ predated the resolution/* adapter/contract/resolve split, so it was structurally stale, not just missing new packages — e.g. resolution/build.md was a flat file where the source is now a resolution/build/ directory of sub-packages. Regenerated wholesale via the same gomarkdoc invocation code-docs.yml uses, rather than patching in just the newest packages, since the drift went back further than this session's changes. Picks up: the reconcile/ and strategy/ split, pkg/apis/serviceunit, pkg/intent/{deployment,serviceunit,package}, core's per-file docs, and the resolution/*/{adapter,contract,resolve} layout. Drops docs for deleted code: pkg/apis/deployment/api/application/ and the old pkg/apis/packages/intent location. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * ci(code-docs): commit generated docs via the Git Data API, drop GPG (#285) git commit -S required GPG_PRIVATE_KEY/GPG_PASSPHRASE just to get a "Verified" badge on an automated docs commit. GitHub marks API-created commits as Verified automatically (same mechanism as an edit made in the web UI) when made with an authenticated GitHub App token — which this workflow already generates for checkout. Replaced the GPG-import + `git commit -S && git push` steps with a Git Data API call (getRef/getCommit/createTree/createCommit/updateRef) via actions/github-script, with one retry if main moved underneath it. Verified locally: `act push -j godoc -W .github/workflows/code-docs.yml -e <(echo '{"ref":"refs/heads/main"}') -n` dry-runs the full step sequence cleanly, and the change-detection + tree-building logic was exercised against a real gomarkdoc regen of this repo's docs/code/ (83 changed paths, correctly split into adds/modifies vs. deletions) via a standalone harness with the GitHub API calls stubbed — real API calls weren't made since that would create a real commit. No more GPG_PRIVATE_KEY/GPG_PASSPHRASE dependency for this workflow. APP_ID/APP_PRIVATE_KEY remain required; the App's Contents permission must be Read & write (already relied on for the existing push). Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * test(resolution): add native Go fuzz tests for CR contract resolvers Resolve{GitRepository,Route,Domain} decode attacker-controlled Contract.Raw JSON from CRs and manually walk nested maps/type assertions beyond what encoding/json validates. Fuzzing the existing xWithContract(raw) test helpers exercises that parsing directly, satisfying the OpenSSF Scorecard Fuzzing check via Go's native testing.F support (go.dev/doc/security/fuzz). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * refactor(deployment): split strategy/reconcile dispatch out of api (#289) * refactor(deployment): split strategy/reconcile dispatch out of api api/ was carrying both infra (materializing k8s objects, GitOps commits) and dispatch (which reconciliation mode, which runtime/strategy) in one flat package. Split dispatch into two new packages: - reconcile/: ReconciliationExecutor, the Imperative-vs-GitOps axis. - strategy/: RuntimeProvider + K8SStrategy (the Rolling/BlueGreen switch pulled out of K8SProvider.Execute), plus the ECS/Knative placeholder reconcilers. Kubernetes, ECS, and Knative are deployment strategies in this domain, not a separate runtime layer, so they live together here. api/ keeps only the infra that actually materializes objects: K8SProvider's apply/teardown (ApplyServiceUnit now exported for strategy.K8SStrategy to call) and KustomizeStrategyProvider's GitOps commit path. Also persists the go-run-driver verification recipe used to confirm this and the earlier structural cleanup didn't regress the Deployment pipeline. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(deployment): cover the api/application/strategy/reconcile split Adds tests for the packages touched by the reconcile/strategy split: K8SProvider's apply/teardown infra and ProviderRegistry (api), DeploymentService.Reconcile end-to-end and StatusWriter (application), ReconciliationExecutor's imperative/GitOps dispatch (reconcile), RuntimeProvider + deriveDeploymentPhase (strategy), and the Deployment/ Service builders (render/builders) — all previously at 0%. pkg/apis/deployment/application/mapper.go is left untested: Mapper / MapResolvedToDomain have no callers anywhere (DeploymentService.Reconcile goes through IntentBuilder, not this Mapper), so it's dead code rather than a coverage gap — flagged separately for a cleanup follow-up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(deployment,intent): add package doc comments api, application, domain, reconcile, strategy (pkg/apis/deployment) and pkg/intent/{deployment,serviceunit,package} had no package-level doc comment at all — browsing the freshly regenerated docs/code output made this obvious, since these packages rendered with no overview text next to ones that do have it (e.g. pkg/apis/serviceunit). domain/model.go's existing "DOMAIN PRINCIPLES" block was mid-file, after the package clause, so godoc/gomarkdoc never picked it up as the package doc — folded into a proper package comment instead of leaving a duplicate. pkg/intent/deployment's comment also documents in one place what was only previously flagged in a private note: ResolveDeploymentIntent (resolve.go) is a superseded, partial constructor with no real callers — IntentBuilder.Build is the one DeploymentService actually uses. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * feat(environment): provision Vault ClusterSecretStore/SecretStore; expand fuzz coverage SecretStoreReconciler provisions the ESO ClusterSecretStore/SecretStore backing an Environment's declared secretStore.provider, dispatched by provider string with a builder per provider (vault.go fully implemented via Kubernetes auth; aws/azure/gcp stubbed pending their own ResolvedXConfig). Unlike the create-once composed-CR secret reconcilers under pkg/secrets, this always server-side-applies the desired spec so it converges on later changes, matching K8SProvider.applyDeployment's pattern. resolve.go validates contract.secretStore.vault (address/path/role required; mountPath/version defaulted) at resolution time rather than deep inside ESO reconciliation. Also adds native Go fuzz targets (testing.F) for the five remaining CR contract resolvers that decode attacker-controlled Contract.Raw JSON (Build, Deployment, Environment, GitHubEvent, Package), rounding out the OpenSSF Scorecard Fuzzing coverage started on fix/native-go-fuzzing to every Resolve* that parses a raw JSON contract. fuzzing.yml's matrix now runs all eight targets. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: github-actions <actions@github.com>
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.
What
Why
Domain
environmentseventssourcesnetworkscommonAPI impact
v1alpha1— free to changev1beta1— backwards-compatible only, deprecations allowedv1— breaking change (requires version bump + changelog entry + migration note)Checklist
mage verifypasses locallybuf breakingreviewed (failures justified above if pre-v1)panic()calls in resolution or domain layersgen/go/blanketops/...for contract typesenvironments.blanketops.dev/*)core.SetConditionat each domain pipeline stagecore.EventRecorderfor terminal outcomesNotes for reviewer