diff --git a/.agentready-config.yaml b/.agentready-config.yaml new file mode 100644 index 00000000..44096d26 --- /dev/null +++ b/.agentready-config.yaml @@ -0,0 +1,7 @@ +# AgentReady configuration for ansible-operator-plugins +# See https://github.com/ambient-code/agentready for documentation +# +# openapi_specs is excluded because this project has no public HTTP API surface. +# See docs/decisions/adr-0004-openapi-not-applicable.md. +excluded_attributes: + - openapi_specs diff --git a/.claude/rules/controller.md b/.claude/rules/controller.md new file mode 100644 index 00000000..4f689548 --- /dev/null +++ b/.claude/rules/controller.md @@ -0,0 +1,15 @@ +--- +paths: + - "internal/ansible/controller/**" +--- + +# Controller Module Rules + +- Reconcile loop must update status conditions on every exit path (Running, Failure, or Successful). +- Use `APIReader` (direct API reads, bypassing cache) for status updates to prevent stale writes. +- Controllers are named `--controller` (lowercased), registered via `controller.New`. +- Finalizer lifecycle: added on first reconcile if configured, removed only after a successful finalizer run on deletion. +- The reconciler requires a `playbook_on_stats` event somewhere in the event stream (recorded as seen, then verified after the stream closes); if missing, reconciliation fails. +- `SetCondition` is a no-op when Type/Status/Reason are unchanged, except `FailureConditionType` which always updates. +- Per-CR reconcile period override via annotation: `ansible.sdk.operatorframework.io/reconcile-period: `. +- Validate at package level: `go test ./internal/ansible/controller/ -short`, `go vet ./internal/ansible/controller/`. diff --git a/.claude/rules/downstream.md b/.claude/rules/downstream.md new file mode 100644 index 00000000..74c467b0 --- /dev/null +++ b/.claude/rules/downstream.md @@ -0,0 +1,15 @@ +--- +paths: + - "openshift/**" +--- + +# Downstream (OpenShift) Module Rules + +- The `openshift/` directory is an independent build overlay with its own `go.mod`, `vendor/`, `Makefile`, and `Dockerfile`. +- `openshift/` Go code does not import the root module's packages at build time (separate module). +- Commits during downstream rebases MUST use `UPSTREAM: :` or `UPSTREAM: :` prefix convention. +- DO NOT hand-edit `openshift/vendor/` or `openshift/release/ansible/ansible_collections/` -- these are generated. +- Downstream Make targets: `update-collections`, `generate-requirements`, `check-requirements`, `check-collections`. +- Dependency updates may need to happen in both `go.mod` and `openshift/go.mod`. +- Rebase workflow uses `openshift/hack/rebase_upstream.sh`. +- See `docs/references/downstream-sync.md` for the full rebase workflow. diff --git a/.claude/rules/proxy.md b/.claude/rules/proxy.md new file mode 100644 index 00000000..297b450c --- /dev/null +++ b/.claude/rules/proxy.md @@ -0,0 +1,16 @@ +--- +paths: + - "internal/ansible/proxy/**" +--- + +# Proxy Module Rules + +- The proxy binds to `localhost` only. DO NOT change the bind address -- it has no authentication of its own. +- DO NOT remove either `Authorization` header stripping call in the handler chain. Both are required. +- Handler chain is assembled inside-out; ordering is load-bearing. +- Adding middleware that modifies request bodies must go between the authorization-stripping and owner-injection layers, never outside the cache handler. +- The cache handler implements a 6-second timeout with fallback to the API server. +- Owner references are injected via the proxy to track dependent resources for garbage collection. +- Metrics API binds to `localhost:5050`; do not expose externally. +- HTTP/2 is disabled by default (`--enable-http2=false`); `ReadHeaderTimeout: 5s` on all HTTP servers. +- Validate at package level: `go test ./internal/ansible/proxy/ -short`. diff --git a/.claude/rules/runner.md b/.claude/rules/runner.md new file mode 100644 index 00000000..2da1f04c --- /dev/null +++ b/.claude/rules/runner.md @@ -0,0 +1,16 @@ +--- +paths: + - "internal/ansible/runner/**" +--- + +# Runner Module Rules + +- `ansible-runner` must be on `$PATH`. Detected via `exec.LookPath` at run time, not startup. +- Input directory layout: `/tmp/ansible-operator/runner//////`. +- Parameters in `env/extravars` are snake_cased from the CR spec. The `markUnsafe` feature wraps strings as `{"__ansible_unsafe": ""}`. +- Each reconcile creates a Unix socket at `/tmp/ansibleoperator-` for the event API HTTP server. +- Events channel is buffered with capacity 1000 and a 10-second write timeout to prevent blocking. +- Status events (those without a UUID) are silently dropped; only JobEvents with a UUID are forwarded. +- After each run, a `latest` symlink is created under `artifacts/`. +- Use `sync.RWMutex` for concurrent data structures; never use `sync.Map`. +- Validate at package level: `go test ./internal/ansible/runner/ -short`. diff --git a/.claude/rules/scaffold.md b/.claude/rules/scaffold.md new file mode 100644 index 00000000..7c9b0a77 --- /dev/null +++ b/.claude/rules/scaffold.md @@ -0,0 +1,14 @@ +--- +paths: + - "pkg/plugins/ansible/v1/**" +--- + +# Scaffold/Plugin Module Rules + +- Templates in `pkg/plugins/ansible/v1/scaffolds/internal/templates/` are the source of truth for scaffolded operator projects. +- After modifying any template, always run `make generate` to regenerate `testdata/`. +- DO NOT hand-edit files in `testdata/` -- they are generated artifacts. +- `pkg/` must never import `internal/`. The plugin scaffolding package is a public API consumed by downstream projects. +- Scaffold output includes: Dockerfile, watches.yaml, Makefile, roles directory, molecule tests. +- Pod security defaults: `runAsNonRoot`, `seccompProfile: RuntimeDefault`, drops all capabilities. +- Validate after template changes: run `make generate`, review the intended output with `git diff -- testdata/` (a diff here is expected for a real template change, not a failure), commit the template and generated-output changes together, then run `make verify`. diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000..1d4acb61 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,26 @@ +{ + "hooks": { + "PostToolUse": [ + { + "matcher": "Edit|Write", + "hooks": [ + { + "type": "command", + "command": "if [[ \"$CLAUDE_FILE_PATH\" == *.go ]]; then gofmt -w \"$CLAUDE_FILE_PATH\" 2>/dev/null; fi" + } + ] + } + ], + "PreToolUse": [ + { + "matcher": "Shell", + "hooks": [ + { + "type": "command", + "command": "echo \"$CLAUDE_TOOL_INPUT\" | grep -qE '(rm -rf /|git push.*--force.*main|git reset --hard)' && echo 'BLOCK: destructive operation detected' && exit 1 || true" + } + ] + } + ] + } +} diff --git a/.claude/skills/add-controller-feature/SKILL.md b/.claude/skills/add-controller-feature/SKILL.md new file mode 100644 index 00000000..461729d8 --- /dev/null +++ b/.claude/skills/add-controller-feature/SKILL.md @@ -0,0 +1,58 @@ +# Add a Controller Reconcile Feature + +## When to Use + +When adding new behavior to the reconciliation loop, such as new status +conditions, event handling, or ansible-runner interaction patterns. + +## Steps + +1. **Identify the reconcile stage** where the feature belongs. The reconcile + loop in `internal/ansible/controller/reconcile.go` follows this flow: + - Read CR via `APIReader` + - Check finalizer lifecycle + - Start ansible-runner subprocess + - Consume events from the event API + - Update status conditions + +2. **Implement the feature** in the appropriate file: + - `reconcile.go` -- main reconcile logic + - `status/` -- status condition management + - `controller.go` -- controller setup and watches + +3. **Add tests** alongside source in the same package: + +```bash +go test ./internal/ansible/controller/ -run TestMyFeature -short +``` + +## Key Rules + +- Reconcile loop MUST update status conditions on every exit path. +- Use `APIReader` for status updates to prevent stale writes from cached data. +- Three condition types: `Running`, `Failure`, `Successful` -- transition between them. +- Use `sync.RWMutex` for concurrent data structures; never `sync.Map`. +- Log messages: Error/Fatal/Info/Warn must begin with uppercase. Error messages + (`errors.New`, `fmt.Errorf`) must begin with lowercase and not end with a period. +- Logging: use `logf.Log.WithName("...")`. V(0) for lifecycle, V(1) for events, V(2) for bodies. + +## Reference Files + +- Reconcile loop: `internal/ansible/controller/reconcile.go` +- Status management: `internal/ansible/controller/status/` +- Controller setup: `internal/ansible/controller/controller.go` + +## Design Doc Enforcement + +If this change affects reconcile flow, event handling, or component +boundaries, review and update the architecture doc in +`docs/architecture/components.md` in the same PR so the design stays +accurate for future agents. + +## Validation + +```bash +go vet ./internal/ansible/controller/ +go test ./internal/ansible/controller/ -short +make verify +``` diff --git a/.claude/skills/add-watch-entry/SKILL.md b/.claude/skills/add-watch-entry/SKILL.md new file mode 100644 index 00000000..40662c7e --- /dev/null +++ b/.claude/skills/add-watch-entry/SKILL.md @@ -0,0 +1,41 @@ +# Add a New GVK Watch Entry + +## When to Use + +When adding support for a new Kubernetes resource (Group/Version/Kind) to be +managed by an Ansible playbook or role. + +## Steps + +1. **Define the watch** in `watches.yaml`: + +```yaml +- group: example.com + version: v1alpha1 + kind: MyResource + role: roles/myresource + # or: playbook: playbooks/myresource.yml +``` + +2. **Create the Ansible role** under `roles/myresource/` with standard structure + (`tasks/main.yml`, `defaults/main.yml`, etc.). + +3. **Reference files**: + - Watch schema and defaults: `internal/ansible/watches/watches.go` + - Example watch config: `testdata/ansible/memcached-operator/watches.yaml` + - Watch loading/validation: `internal/ansible/watches/watches_test.go` + +## Key Rules + +- A watch must specify exactly one of `playbook` or `role`, never both. +- Default values: `manageStatus: true`, `watchDependentResources: true`, + `snakeCaseParameters: true`, `maxRunnerArtifacts: 20`, `ansibleVerbosity: 2`. +- Per-GVK concurrency: set via `MAX_CONCURRENT_RECONCILES__` env var. +- Supports `${VAR}` environment variable interpolation via `os.Expand`. + +## Validation + +```bash +go test ./internal/ansible/watches/ -short +make verify +``` diff --git a/.claude/skills/downstream-carry/SKILL.md b/.claude/skills/downstream-carry/SKILL.md new file mode 100644 index 00000000..86aa60ed --- /dev/null +++ b/.claude/skills/downstream-carry/SKILL.md @@ -0,0 +1,43 @@ +# Downstream Carry Patch + +## When to Use + +When making a change that is specific to the OpenShift downstream fork and must +persist across upstream rebases. + +## Steps + +1. **Make changes** in the `openshift/` directory (it has its own `go.mod`, + `vendor/`, `Makefile`, and `Dockerfile`). + +2. **Commit with the carry prefix**: + +```bash +git commit -m "UPSTREAM: : description of the change" +``` + +3. **Validate downstream targets**: + +```bash +cd openshift && make check-requirements check-collections +``` + +## Commit Prefix Convention + +| Prefix | Meaning | +|---|---| +| `UPSTREAM: :` | Preserve this change across future rebases | +| `UPSTREAM: :` | Accept upstream version; discard this delta on next rebase | + +## Key Rules + +- `openshift/` Go code does not import the root module's packages at build time. +- DO NOT hand-edit `openshift/vendor/` or `openshift/release/ansible/ansible_collections/`. +- Rebase workflow: `openshift/hack/rebase_upstream.sh`. +- If a carry patch touches both root and `openshift/`, split into separate commits. + +## Reference + +- Full rebase workflow: `docs/references/downstream-sync.md` +- Downstream overview: `openshift/README.md` +- ADR: `docs/decisions/adr-0001-upstream-downstream-mirror.md` diff --git a/.claude/skills/scaffold-template/SKILL.md b/.claude/skills/scaffold-template/SKILL.md new file mode 100644 index 00000000..54bfc253 --- /dev/null +++ b/.claude/skills/scaffold-template/SKILL.md @@ -0,0 +1,54 @@ +# Modify Scaffold Templates + +## When to Use + +When changing the generated output of `ansible-operator init` or +`ansible-operator create api` -- the files scaffolded for new operator projects. + +## Steps + +1. **Edit templates** in `pkg/plugins/ansible/v1/scaffolds/internal/templates/`. + These are Go template files that produce the scaffolded project structure. + +2. **Rebuild the binary**: + +```bash +make build +``` + +3. **Regenerate testdata** (this runs the scaffolder against sample inputs): + +```bash +make generate +``` + +4. **Verify no unintended changes**: + +```bash +git diff testdata/ +make verify +``` + +## Key Rules + +- DO NOT hand-edit files in `testdata/` -- they are generated from templates. +- `pkg/` must never import `internal/`. The plugin package is a public API. +- Scaffold output includes: Dockerfile, watches.yaml, Makefile, roles directory, + molecule tests, RBAC manifests. +- Pod security defaults in scaffolded output: `runAsNonRoot`, + `seccompProfile: RuntimeDefault`, drops all capabilities. +- Every new `.go` file must have an Apache 2.0 license header. + +## Reference Files + +- Template directory: `pkg/plugins/ansible/v1/scaffolds/internal/templates/` +- Plugin entry point: `pkg/plugins/ansible/v1/init.go` +- Generated samples: `testdata/memcached-molecule-operator/` + +## Validation + +```bash +make generate +git diff -- testdata/ # review the generated diff; expected for intended template changes +make verify +``` diff --git a/.claude/skills/update-dependencies/SKILL.md b/.claude/skills/update-dependencies/SKILL.md new file mode 100644 index 00000000..a9fd35a3 --- /dev/null +++ b/.claude/skills/update-dependencies/SKILL.md @@ -0,0 +1,51 @@ +# Update Go Dependencies + +## When to Use + +When bumping a Go module dependency version, adding a new dependency, or +responding to a Dependabot alert. + +## Steps + +1. **Update `go.mod`**: + +```bash +go get @ +``` + +2. **Tidy and vendor**: + +```bash +go mod tidy +go mod vendor +``` + +3. **Run full validation**: + +```bash +make verify +``` + +4. **Check for dirty tree** (CI will fail if vendor is stale): + +```bash +git diff --exit-code vendor/ +``` + +## Key Rules + +- This project vendors all dependencies. Always commit the updated `vendor/`. +- `make fix` runs `go mod tidy` but does NOT run `go mod vendor`. +- Dependency updates may need to happen in both `go.mod` (root) and + `openshift/go.mod` (downstream overlay). +- Key dependencies to be careful with: `controller-runtime`, `client-go`, + `operator-lib`, `kubebuilder/v4`. +- Tool versions are managed by bingo in `.bingo/` -- update those separately. + +## Validation + +```bash +go mod tidy && go mod vendor +make verify +git diff --exit-code +``` diff --git a/.coderabbit.yaml b/.coderabbit.yaml new file mode 100644 index 00000000..18768316 --- /dev/null +++ b/.coderabbit.yaml @@ -0,0 +1,10 @@ +# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json +# See https://docs.coderabbit.ai/reference/configuration for all fields and default values +knowledge_base: + code_guidelines: + filePatterns: + - "docs/domain/*.md" + - "docs/architecture/*.md" + - "docs/references/*.md" + - "docs/AOP_DEVELOPMENT.md" + - "docs/AOP_TESTING.md" diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 00000000..49cc9019 --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,41 @@ +name: CodeQL + +on: + push: + branches: [main] + pull_request: + branches: [main] + schedule: + - cron: '0 6 * * 1' + +jobs: + analyze: + name: CodeQL Analysis + runs-on: ubuntu-22.04 + permissions: + security-events: write + actions: read + contents: read + strategy: + fail-fast: false + matrix: + language: [go] + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: Setup Go + uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 + with: + go-version-file: "go.mod" + - name: Initialize CodeQL + uses: github/codeql-action/init@faaca9a8f6edddba5725ffe5adefdab6669a2eca # v3.38.0 + with: + languages: ${{ matrix.language }} + - name: Build + run: make build + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@faaca9a8f6edddba5725ffe5adefdab6669a2eca # v3.38.0 + with: + category: "/language:${{ matrix.language }}" diff --git a/.github/workflows/semgrep.yml b/.github/workflows/semgrep.yml new file mode 100644 index 00000000..4e67993a --- /dev/null +++ b/.github/workflows/semgrep.yml @@ -0,0 +1,30 @@ +# Semgrep: multi-rule static analysis (SAST) complementing CodeQL. +# Uses the community "auto" ruleset, which covers Go security and +# correctness patterns (no API token required). +# +# Runs directly on the host runner (not the semgrep/semgrep container image, +# which defaults to root with no non-root USER) and installs a pinned +# semgrep version via pip. +name: Semgrep + +on: + push: + branches: [main] + pull_request: + branches: [main] + +jobs: + semgrep: + name: Semgrep SAST scan + runs-on: ubuntu-22.04 + permissions: + contents: read + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: Install Semgrep + run: pip install 'semgrep==1.176.1' + - name: Run Semgrep + run: semgrep scan --config=p/ci --config=p/golang --error --exclude vendor --exclude openshift/vendor --exclude testdata diff --git a/.github/workflows/test-ansible.yml b/.github/workflows/test-ansible.yml index 53309a55..1abf60fa 100644 --- a/.github/workflows/test-ansible.yml +++ b/.github/workflows/test-ansible.yml @@ -1,3 +1,6 @@ +# Ansible E2E workflow: runs on every PR. +# Validates: end-to-end tests requiring Docker/Kind cluster. +# Make targets: make test-e2e-ansible, make test-e2e-ansible-molecule name: ansible on: pull_request: {} diff --git a/.github/workflows/test-sanity.yml b/.github/workflows/test-sanity.yml index a496a9a3..3e15a14d 100644 --- a/.github/workflows/test-sanity.yml +++ b/.github/workflows/test-sanity.yml @@ -1,3 +1,7 @@ +# Sanity workflow: runs on every PR. +# Validates: formatting, generation, lint, vet, license headers, +# error message format, and clean working tree. +# Make target: make test-sanity (part of make verify / make test-static) name: sanity on: pull_request: {} @@ -6,13 +10,19 @@ jobs: sanity: name: sanity runs-on: ubuntu-22.04 + permissions: + contents: read steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: fetch-depth: 0 - - uses: actions/setup-go@v6 + persist-credentials: false + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 with: go-version-file: "go.mod" id: go - run: sudo rm -rf /usr/local/bin/kustomize - - run: make test-sanity + - name: Type check + run: go vet ./... + - name: Run sanity checks + run: make test-sanity diff --git a/.github/workflows/unit.yml b/.github/workflows/unit.yml index 8e90e444..066166e7 100644 --- a/.github/workflows/unit.yml +++ b/.github/workflows/unit.yml @@ -1,3 +1,6 @@ +# Unit workflow: runs on every PR. +# Validates: unit tests with envtest and -short flag. +# Make target: make test-unit (part of make verify / make test-static) name: unit on: pull_request: {} @@ -6,11 +9,28 @@ jobs: unit: name: unit runs-on: ubuntu-22.04 + permissions: + contents: read steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: fetch-depth: 0 - - uses: actions/setup-go@v6 + persist-credentials: false + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 with: + cache: true go-version-file: "go.mod" - - run: make test-unit + - name: Run unit tests with coverage + run: make test-unit + - name: Checksum coverage report + if: always() + run: sha256sum coverage.out > coverage.out.sha256 + - name: Upload coverage report + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: coverage-report + path: | + coverage.out + coverage.out.sha256 + retention-days: 14 diff --git a/.gitignore b/.gitignore index f9f00bba..6e499af6 100644 --- a/.gitignore +++ b/.gitignore @@ -129,5 +129,27 @@ tags # End of https://www.toptal.com/developers/gitignore/api/go,vim,emacs,visualstudiocode -# Python cache (Ansible molecule) +# Python cache and test artifacts (Ansible molecule) test/ansible/plugins/filter/__pycache__/ +__pycache__/ +*.py[cod] +.pytest_cache/ +.mypy_cache/ +.venv/ +htmlcov/ +.coverage +*.egg-info/ + +# Additional coverage and artifact patterns +*.swp +*.swo +.env +.env.* +cover.out +coverage.txt +*.log +tmp/ + +# Go workspace file +go.work +go.work.sum diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..dd588f72 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,63 @@ +linters: + enable: + - govet + - errcheck + - staticcheck + - unused + - ineffassign + - gosimple + - revive + - gosec + - depguard + +linters-settings: + depguard: + rules: + prevent-vendor-import: + deny: + - pkg: "github.com/operator-framework/ansible-operator-plugins/vendor" + desc: "Do not import from vendor directory directly" + prevent-test-in-prod: + files: + - "!$test" + - "!**/test/**" + deny: + - pkg: "github.com/stretchr/testify" + desc: "testify is for tests only" + revive: + rules: + - name: exported + severity: warning + - name: blank-imports + - name: context-as-argument + - name: dot-imports + - name: error-return + - name: error-naming + - name: increment-decrement + - name: var-naming + - name: range + - name: receiver-naming + +issues: + exclude-dirs: + - vendor + - testdata + - openshift/vendor + - openshift/release + exclude-rules: + # Dot-importing ginkgo/gomega is the established, repo-wide test convention + # (16+ existing _test.go files, plus internal/testutils/ helpers, use this + # pattern). Not a real issue. + - path: "(_test\\.go$|^internal/testutils/)" + linters: [revive] + text: "^dot-imports:" + # pkg/testutils is a public API consumed by downstream projects (see AGENTS.md). + # Renaming exported symbols to satisfy naming/stutter conventions would be a + # breaking change; only unexported findings in this path get fixed for real. + - path: "^pkg/testutils/" + linters: [revive] + text: "^(exported|var-naming):" + # gosec is valuable on production controller/runner/proxy code but noisy on + # test helpers, codegen tooling, and e2e utilities where inputs are trusted. + - path: "^(hack/|pkg/testutils/|test/|internal/testutils/)" + linters: [gosec] diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 00000000..48524e37 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,23 @@ +repos: + - repo: https://github.com/dnephin/pre-commit-golang + rev: fb24a639f7c938759fe56eeebbb7713b69d60494 # v0.5.1 + hooks: + - id: go-fmt + - id: go-vet + - repo: https://github.com/golangci/golangci-lint + rev: 89476e7a1eaa0a8a06c17343af960a5fd9e7edb7 # v1.62.2 + hooks: + - id: golangci-lint + args: [--timeout=5m] + - repo: https://github.com/compilerla/conventional-pre-commit + rev: 5f9c312d9978fbcee1da97154ba385834eeda799 # v4.0.0 + hooks: + - id: conventional-pre-commit + stages: [commit-msg] + args: [feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert] + - repo: https://github.com/Yelp/detect-secrets + rev: 01886c8a910c64595c47f186ca1ffc0b77fa5458 # v1.5.0 + hooks: + - id: detect-secrets + args: [--baseline, .secrets.baseline] + exclude: ^(vendor/|openshift/vendor/|testdata/) diff --git a/.secrets.baseline b/.secrets.baseline new file mode 100644 index 00000000..a33641ed --- /dev/null +++ b/.secrets.baseline @@ -0,0 +1,109 @@ +{ + "version": "1.5.0", + "plugins_used": [ + { + "name": "ArtifactoryDetector" + }, + { + "name": "AWSKeyDetector" + }, + { + "name": "AzureStorageKeyDetector" + }, + { + "name": "Base64HighEntropyString", + "limit": 4.5 + }, + { + "name": "BasicAuthDetector" + }, + { + "name": "CloudantDetector" + }, + { + "name": "GitHubTokenDetector" + }, + { + "name": "HexHighEntropyString", + "limit": 3.0 + }, + { + "name": "IbmCloudIamDetector" + }, + { + "name": "IbmCosHmacDetector" + }, + { + "name": "JwtTokenDetector" + }, + { + "name": "KeywordDetector", + "keyword_exclude": "" + }, + { + "name": "MailchimpDetector" + }, + { + "name": "NpmDetector" + }, + { + "name": "PrivateKeyDetector" + }, + { + "name": "SendGridDetector" + }, + { + "name": "SlackDetector" + }, + { + "name": "SoftlayerDetector" + }, + { + "name": "SquareOAuthDetector" + }, + { + "name": "StripeDetector" + }, + { + "name": "TwilioKeyDetector" + } + ], + "filters_used": [ + { + "path": "detect_secrets.filters.allowlist.is_line_allowlisted" + }, + { + "path": "detect_secrets.filters.common.is_ignored_due_to_verification_policies", + "min_level": 2 + }, + { + "path": "detect_secrets.filters.heuristic.is_indirect_reference" + }, + { + "path": "detect_secrets.filters.heuristic.is_likely_id_string" + }, + { + "path": "detect_secrets.filters.heuristic.is_lock_file" + }, + { + "path": "detect_secrets.filters.heuristic.is_not_alphanumeric_string" + }, + { + "path": "detect_secrets.filters.heuristic.is_potential_uuid" + }, + { + "path": "detect_secrets.filters.heuristic.is_prefixed_with_dollar_sign" + }, + { + "path": "detect_secrets.filters.heuristic.is_sequential_string" + }, + { + "path": "detect_secrets.filters.heuristic.is_swagger_file" + }, + { + "path": "detect_secrets.filters.heuristic.is_templated_secret" + } + ], + "results": {}, + "generated_at": "2026-09-10T00:00:00Z" +} diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000..d7f9f7e6 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,141 @@ +# Ansible Operator Plugins - Agentic Documentation + +**Component**: Ansible Operator Plugins (AOP) +**Repository**: openshift/ansible-operator-plugins (downstream mirror of operator-framework/ansible-operator-plugins) + +> **AI agents**: Read `docs/domain/` first for API contracts and watches.yaml schema, +> then `docs/architecture/` for reconcile/runner/proxy implementation patterns. +> Check `docs/decisions/` before making architectural changes. + +## What is Ansible Operator Plugins? + +A Go library and binary that bridges Ansible automation with Kubernetes controllers via controller-runtime. Part of the Operator SDK ecosystem, consumed as a standalone `ansible-operator` binary and as a library by downstream projects (notably the OpenShift fork under `openshift/`). + +**Core loop**: a `watches.yaml` file maps Kubernetes GVKs to Ansible playbooks or roles. For each watch entry, a controller-runtime controller is created. On reconcile, the controller shells out to `ansible-runner`, consumes job events via a Unix-domain-socket HTTP API, and updates the CR's status conditions. + +## Core Components + +| Component | Location | Purpose | +|---|---|---| +| Controller | `internal/ansible/controller/` | Controller setup, reconcile loop, status conditions | +| Runner | `internal/ansible/runner/` | ansible-runner subprocess management, event API | +| Proxy | `internal/ansible/proxy/` | REST proxy intercepting Ansible's K8s API calls | +| Watches | `internal/ansible/watches/` | watches.yaml loading and validation | +| Metrics | `internal/ansible/metrics/`, `internal/ansible/apiserver/` | Prometheus metrics, user-metric API (port 5050) | +| Plugin/Scaffold | `pkg/plugins/ansible/v1/` | Kubebuilder plugin: scaffolding templates | +| Test utilities | `pkg/testutils/` | Public E2E helpers (Kind, kubectl, operator lifecycle) | +| Downstream | `openshift/` | OpenShift fork overlay (separate go.mod, vendor, Makefile) | +| Testdata | `testdata/` | Generated sample operator projects (do not hand-edit) | + +## Critical Patterns + +1. **DO NOT hand-edit** `testdata/`, `vendor/`, or generated scaffold output. Run `make generate` after scaffold/plugin changes; run `go mod tidy && go mod vendor` after dependency changes. +2. **DO NOT type-check a single Go file independently** of its package. Always lint and test at the package level: `go test ./internal/ansible/controller/`. +3. **DO NOT run E2E tests** without Docker and Kind. E2E creates a cluster and builds images. +4. **DO NOT change the proxy bind address** from localhost. The proxy has no authentication of its own. +5. **DO NOT remove either `Authorization` header stripping call** in the proxy handler chain. Both are required. +6. **DO NOT use `sync.Map`** -- the codebase uses `sync.RWMutex` exclusively for concurrent data structures. + +## Design Rationale (selected) + +- `openshift/` uses a separate `go.mod` and vendor tree instead of a Go + workspace, because downstream CVE backports and release cadence must not + block on upstream merges (`docs/decisions/adr-0001-upstream-downstream-mirror.md`). +- Ansible job events are consumed over a Unix-domain-socket HTTP API rather + than parsed from stdout, because structured JSON events are the only + reliable way to drive status conditions (`docs/architecture/components.md`). + +## Single-File Verification + +Go is type-checked at the **package** level. Always validate the enclosing package: + +| Task | Command | +|---|---| +| Lint one package | `golangci-lint run ./internal/ansible/controller/` | +| Type check (vet) | `go vet ./internal/ansible/controller/` | +| Run one test | `go test ./internal/ansible/controller/ -run TestReconcile` | +| Format | `go fmt ./internal/ansible/controller/` | +| Shell check | `shellcheck path/to/script.sh` | +| YAML lint | `yamllint path/to/file.yaml` | + +## Documentation Structure + +```text +docs/ +├── domain/ +│ ├── watches-and-contracts.md # watches.yaml schema, status conditions, annotations, ports +│ └── generated-artifacts.md # testdata, vendor, scaffold output rules +├── architecture/ +│ ├── components.md # Reconcile flow, runner, proxy, event handlers, metrics +│ ├── error-handling.md # Reconciler error contract, status marking, HTTP errors +│ ├── performance.md # Channel buffering, concurrency, mutexes, goroutines +│ └── boundaries.md # Package visibility, dependency direction +├── decisions/ +│ ├── adr-0001-upstream-downstream-mirror.md +│ ├── adr-0002-generated-vendor-artifact-policy.md +│ ├── adr-0003-release-rebase-workflow.md +│ ├── adr-0004-openapi-not-applicable.md +│ └── adr-template.md +├── references/ +│ ├── ecosystem.md # Links to operator-sdk, enhancements, platform patterns +│ ├── downstream-sync.md # UPSTREAM: carry/drop, rebase workflow, openshift/ targets +│ └── security.md # Proxy auth, kubeconfig, RBAC, input validation, file perms +├── AOP_DEVELOPMENT.md # Build, setup, verify, validation matrix, common mistakes +└── AOP_TESTING.md # Ginkgo/testify conventions, envtest, E2E, short mode +``` + +**AI Agent Path**: `docs/domain/` → `docs/architecture/` → `docs/decisions/` → `docs/AOP_DEVELOPMENT.md` or `docs/AOP_TESTING.md` (as relevant) + +## Quick Reference + +| Action | Command | +|---|---| +| Bootstrap tools | `make setup` | +| Full non-cluster validation | `make verify` | +| Build binary | `make build` | +| Unit tests (envtest) | `make test-unit` | +| Sanity (lint, vet, license) | `make test-sanity` | +| E2E (Ansible) | `make test-e2e-ansible` | +| Auto-fix formatting | `make fix` | +| Regenerate testdata | `make generate` | +| Build Docker image | `make image-build` | + +**Framework**: controller-runtime v0.21.0 | **Go**: 1.26.3 | **Module**: `github.com/operator-framework/ansible-operator-plugins` + +## Pattern References + +- **New GVK watch**: follow the pattern in `internal/ansible/watches/watches.go` and `testdata/memcached-molecule-operator/watches.yaml` +- **Controller reconcile feature**: see `internal/ansible/controller/reconcile.go` as reference +- **Scaffold template change**: follow the pattern in `pkg/plugins/ansible/v1/scaffolds/internal/templates/` +- **Go dependency update**: see `go.mod` and `go.sum` as reference; run `go mod tidy && go mod vendor`, then `make verify` +- **Downstream carry patch**: follow the pattern in `openshift/Makefile` and `openshift/hack/rebase_upstream.sh`; prefix commits with `UPSTREAM: :` + +See `docs/patterns/README.md` for the full pattern index and `examples/` for a runnable sample operator. + +## Knowledge Graph + +```text + [AGENTS.md] ← Start here + │ + ┌───────────────┼───────────────┐ + │ │ │ + [docs/domain/] [docs/architecture/] [docs/decisions/] + watches.yaml Reconcile flow ADR history + API contracts Runner/Proxy (4 ADRs) + Generated files Error handling + │ │ │ + └───────────────┼───────────────┘ + │ + [docs/AOP_DEVELOPMENT.md] + [docs/AOP_TESTING.md] + │ + [docs/references/] + Ecosystem links + Downstream sync + Security rules +``` + +## External References + +- [openshift/README.md](openshift/README.md) -- downstream sync walkthrough +- [images/ansible-operator/README.md](images/ansible-operator/README.md) -- container image and Python conventions diff --git a/Makefile b/Makefile index 4b3ac773..6775a7ab 100644 --- a/Makefile +++ b/Makefile @@ -46,6 +46,17 @@ include .bingo/Variables.mk BUILD_GOOS ?= $(shell go env GOOS) BUILD_GOARCH ?= $(shell go env GOARCH) +##@ Setup and Verification + +.PHONY: setup +setup: $(GOLANGCI_LINT) $(SETUP_ENVTEST) $(KIND) ## Bootstrap dev tools via bingo (idempotent, no cluster creation) + +.PHONY: verify +verify: test-static ## Full non-cluster validation (CI-equivalent: sanity + unit) + +.PHONY: check +check: verify ## Alias for verify + ##@ Development .PHONY: generate @@ -120,11 +131,13 @@ test-sanity: generate fix ## Test repo formatting, linting, etc. make lint git diff --exit-code # diff again to ensure other checks don't change repo -.PHONY: test-docs -test-docs: ## Test doc links - go run ./release/changelog/gen-changelog.go -validate-only - git submodule update --init --recursive website/ - ./hack/check-links.sh +# test-docs is disabled: requires missing release/changelog/gen-changelog.go, +# website/ submodule, and hack/check-links.sh. Do not add to CI until restored. +# .PHONY: test-docs +# test-docs: ## Test doc links +# go run ./release/changelog/gen-changelog.go -validate-only +# git submodule update --init --recursive website/ +# ./hack/check-links.sh .PHONY: test-unit ENVTEST_VERSION = $(shell go list -m k8s.io/client-go | cut -d" " -f2 | sed 's/^v0\.\([[:digit:]]\{1,\}\)\.[[:digit:]]\{1,\}$$/1.\1.x/') @@ -132,6 +145,10 @@ TEST_PKGS = $(shell go list ./... | grep -v -E 'github.com/operator-framework/an test-unit: $(SETUP_ENVTEST) ## Run unit tests KUBEBUILDER_ASSETS="$(shell $(SETUP_ENVTEST) use $(ENVTEST_VERSION) -p path)" go test -coverprofile=coverage.out -covermode=count -short $(TEST_PKGS) +.PHONY: test-race +test-race: $(SETUP_ENVTEST) ## Run unit tests with race detector + KUBEBUILDER_ASSETS="$(shell $(SETUP_ENVTEST) use $(ENVTEST_VERSION) -p path)" CGO_ENABLED=1 go test -race -short $(TEST_PKGS) + e2e_tests := test-e2e-ansible test-e2e-ansible-molecule e2e_targets := test-e2e $(e2e_tests) .PHONY: $(e2e_targets) diff --git a/README.md b/README.md index 0551dd19..98267775 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,8 @@ # Ansible Operator Plugins -A plugin that provide Ansible-based operator functionality for the [Operator SDK](https://github.com/operator-framework/operator-sdk). This project contains the core Ansible operator implementation that enables developers to build Kubernetes operators using Ansible playbooks and roles. +[![sanity](https://github.com/openshift/ansible-operator-plugins/actions/workflows/test-sanity.yml/badge.svg)](https://github.com/openshift/ansible-operator-plugins/actions/workflows/test-sanity.yml) [![unit](https://github.com/openshift/ansible-operator-plugins/actions/workflows/unit.yml/badge.svg)](https://github.com/openshift/ansible-operator-plugins/actions/workflows/unit.yml) + +A plugin that provides Ansible-based operator functionality for the [Operator SDK](https://github.com/operator-framework/operator-sdk). This project contains the core Ansible operator implementation that enables developers to build Kubernetes operators using Ansible playbooks and roles. ## Overview @@ -8,7 +10,122 @@ This project provides the Ansible plugin for Operator SDK, allowing you to: - Build Kubernetes operators using Ansible playbooks and roles - Manage custom resources with Ansible automation - Handle operator lifecycle events through Ansible tasks -- Leverage the full ecosystem of Ansible modules and collections +- Leverage the full ecosystem of Ansible modules and collections + +## Tech Stack + +| Component | Version / Details | +|---|---| +| Go | 1.26.3 | +| Module path | `github.com/operator-framework/ansible-operator-plugins` | +| Kubernetes libs | k8s.io v0.33.x (`client-go`, `apimachinery`, `api`) | +| Controller framework | [controller-runtime](https://pkg.go.dev/sigs.k8s.io/controller-runtime) v0.21.0 | +| Scaffolding framework | [kubebuilder](https://pkg.go.dev/sigs.k8s.io/kubebuilder/v4) v4.6.0 | +| Operator utilities | [operator-lib](https://github.com/operator-framework/operator-lib) v0.19.0 | +| Metrics | [prometheus/client_golang](https://github.com/prometheus/client_golang) v1.23.2 | +| CLI | cobra v1.10.2, pflag, viper | +| Testing | Ginkgo v2 / Gomega, testify, envtest | +| Tool management | [bingo](https://github.com/bwplotka/bingo) (golangci-lint, goreleaser, kind, setup-envtest) | +| Container image | `quay.io/operator-framework/ansible-operator` | + +## Project Structure + +``` +cmd/ansible-operator/ Single binary entrypoint (cobra CLI) +internal/ + ansible/ + controller/ Controller setup + reconcile loop + runner/ ansible-runner subprocess management + proxy/ REST proxy intercepting Ansible's K8s API calls + watches/ watches.yaml loading and validation + events/ Ansible event logging + metrics/ Prometheus metric definitions + apiserver/ Metrics API server (localhost:5050) + cmd/ansible-operator/run/ "run" subcommand (manager setup, proxy start) + version/ Build-time version variables (ldflags) +pkg/ + plugins/ansible/v1/ Kubebuilder plugin: scaffolding templates + testutils/ Public E2E test utilities +hack/ Scripts for generation, linting, license checks +images/ansible-operator/ Dockerfile + Pipfile for operator image +openshift/ Downstream OpenShift fork overlay +testdata/ Generated sample operator projects (do not hand-edit) +``` + +## Building and Testing + +```sh +# Bootstrap dev tools (idempotent, no cluster creation) +make setup + +# Build the ansible-operator binary +make build + +# Full non-cluster validation (sanity + unit) +make verify + +# Unit tests only (uses envtest, skips E2E) +make test-unit + +# Sanity checks: formatting, linting, vet, license headers, error message format +make test-sanity + +# Full E2E suite (creates a Kind cluster, builds images) +make test-e2e + +# Ansible-specific E2E only +make test-e2e-ansible + +# Auto-fix: go mod tidy + go fmt + golangci-lint --fix +make fix + +# Regenerate testdata after scaffold template changes +make generate +``` + +Cross-compile by setting `BUILD_GOOS` and `BUILD_GOARCH`: + +```sh +BUILD_GOOS=linux BUILD_GOARCH=arm64 make build +``` + +Build the Docker image: + +```sh +make image-build +``` + +### Vendoring + +This project vendors all dependencies. After modifying `go.mod`: + +```sh +go mod tidy +go mod vendor +``` + +Commit the updated `vendor/` directory. The `make test-sanity` target will fail if the working tree is dirty after generation. + +## Upstream/Downstream Synchronization + +The `openshift/` directory contains an independent build overlay for the OpenShift downstream fork. See [openshift/README.md](openshift/README.md) for the rebase walkthrough and [docs/references/downstream-sync.md](docs/references/downstream-sync.md) for the `UPSTREAM: :` commit convention. + +## Further Documentation + +| Document | Description | +|---|---| +| [AGENTS.md](AGENTS.md) | Component overview, AI agent routing, critical patterns | +| [docs/domain/](docs/domain/) | watches.yaml schema, API contracts, generated artifact rules | +| [docs/architecture/](docs/architecture/) | Reconcile flow, runner, proxy, error handling, performance | +| [docs/decisions/](docs/decisions/) | ADRs for upstream/downstream, vendor policy, release workflow | +| [docs/AOP_DEVELOPMENT.md](docs/AOP_DEVELOPMENT.md) | Build, validation matrix, code conventions, common mistakes | +| [docs/AOP_TESTING.md](docs/AOP_TESTING.md) | Ginkgo/testify conventions, envtest, E2E infrastructure | +| [docs/references/](docs/references/) | Ecosystem links, downstream sync, security rules | +| [THREAT_MODEL.md](THREAT_MODEL.md) | Trust boundaries and threat analysis (draft) | + +## Security + +For security vulnerabilities, please see [THREAT_MODEL.md](THREAT_MODEL.md) for the trust boundary analysis and [docs/references/security.md](docs/references/security.md) for codebase security conventions. # Releasing Guide diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 00000000..90886180 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,60 @@ +# Security Policy + +## Reporting a Vulnerability + +The Ansible Operator Plugins project follows the [Operator Framework security +policy](https://github.com/operator-framework/community/blob/main/SECURITY.md). + +**Do not open a public GitHub issue for security vulnerabilities.** + +To report a vulnerability privately, use one of the following: + +- GitHub Security Advisories: open a draft advisory at + https://github.com/operator-framework/ansible-operator-plugins/security/advisories/new +- Email the maintainers listed in [OWNERS](OWNERS), or reach the Operator + Framework working group. + +Please include: + +- A description of the vulnerability and its impact +- Steps to reproduce (proof-of-concept if possible) +- Affected version(s) or commit SHA +- Any known mitigations + +## Supported Versions + +Security fixes are applied to the most recent minor release branch and +backported to the current downstream OpenShift rebase branch (see +`docs/references/downstream-sync.md`). Older branches are not routinely +patched. + +## Scope and Threat Model + +For a structured description of trust boundaries, entry points, and known +threats, see [`THREAT_MODEL.md`](THREAT_MODEL.md). Key security-relevant +areas of this codebase: + +- The Ansible proxy (`internal/ansible/proxy/`) binds to localhost only and + has no authentication of its own — see `docs/references/security.md`. +- The `Authorization` header is stripped in the proxy handler chain; both + stripping calls are required and must not be removed. +- The user-metrics API (`internal/ansible/apiserver/`, port 5050) accepts + local Unix-domain-socket input from `ansible-runner`, not external traffic. + +## Dependency and Static Analysis Scanning + +This repository uses automated scanning to catch known-vulnerable +dependencies and common code security issues before merge: + +- **Dependabot** (`.github/dependabot.yml`) — dependency update alerts +- **CodeQL** (`.github/workflows/codeql.yml`) — static application security + testing (SAST) for Go +- **Semgrep** (`.github/workflows/semgrep.yml`) — multi-rule SAST +- **detect-secrets** (`.pre-commit-config.yaml`) — pre-commit secret scanning + +## Disclosure Process + +1. Report received and acknowledged (best effort within 5 business days). +2. Maintainers validate and assess severity. +3. A fix is developed privately and a coordinated release is prepared. +4. A GitHub Security Advisory is published once a fix is available. diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md new file mode 100644 index 00000000..6c3f86c6 --- /dev/null +++ b/THREAT_MODEL.md @@ -0,0 +1,146 @@ +# Threat Model: Ansible Operator Plugins + +> **Status**: Draft -- pending maintainer and security team review. + +## 1. System context + +Ansible Operator Plugins is a Go library and binary that bridges Ansible +automation with Kubernetes controllers via controller-runtime. The operator +process runs inside a Kubernetes pod, shells out to `ansible-runner` for +playbook/role execution, and communicates with the Kubernetes API server via a +localhost proxy. It manages Custom Resources by mapping GVKs to Ansible +automation defined in a `watches.yaml` file. + +The operator is deployed by the operator author (cluster admin) and executes +playbooks defined at build time -- end users interact only via Custom Resource +CRUD operations. + +## 2. Assets + +| Asset | Description | Sensitivity | +|---|---|---| +| Kubernetes API credentials | ServiceAccount token projected into the pod | Critical | +| Kubeconfig temp files | Per-reconcile kubeconfig written to `/tmp/` | High | +| Custom Resource spec data | User-provided CR fields passed to Ansible as extravars | Medium-High | +| Ansible playbook/role code | Operator logic defined at build time | Medium | +| Status conditions | CR status subresource updated by the reconciler | Medium | +| Metrics data | Prometheus metrics on port 5050 | Low | +| Runner artifacts | Ansible-runner output in `/tmp/ansible-operator/runner/` | Low | + +## 3. Entry points & trust boundaries + +| Entry point | Description | Trust boundary | Reachable assets | +|---|---|---|---| +| Kubernetes API watch | CR create/update/delete events from API server | Cluster RBAC | CR spec data, status | +| Proxy HTTP (localhost) | Ansible modules call K8s API via localhost proxy | Pod-local only | K8s API credentials | +| Event API socket | Unix socket for ansible-runner job events | Pod-local only | Status conditions | +| Metrics API (localhost:5050) | User-defined metrics endpoint | Pod-local only | Metrics data | +| ansible-runner subprocess | Ansible playbook execution via `exec` | Pod process boundary | All pod-accessible resources | +| Container image layers | Python packages, Ansible collections | Build-time supply chain | All runtime assets | + +```text +┌─────────────────────────────────────────────────────────┐ +│ Kubernetes Cluster │ +│ │ +│ ┌─────────────────────────────────────────────────┐ │ +│ │ Operator Pod │ │ +│ │ │ │ +│ │ ┌──────────┐ ┌──────────┐ ┌──────────────┐ │ │ +│ │ │Controller │ │ Proxy │ │ansible-runner│ │ │ +│ │ │ (Go) │──│localhost │──│ (Python) │ │ │ +│ │ └──────────┘ └──────────┘ └──────────────┘ │ │ +│ │ │ │ │ │ +│ │ │ kubeconfig (tmp) │ unix sock │ │ +│ │ │ │ /tmp/ │ │ +│ └───────┼────────────────────────────┼────────────┘ │ +│ │ │ │ +│ K8s API Server Playbook execution │ +│ │ +└─────────────────────────────────────────────────────────┘ +``` + +## 4. Threats + +| ID | Threat | Actor | Impact | Likelihood | Status | +|---|---|---|---|---|---| +| T1 | Overly broad RBAC on scaffolded ClusterRole | Operator author (misconfiguration) | High | Medium | Documented | +| T2 | Stale cache serving outdated data | N/A (race condition) | Medium | Low | Mitigated | +| T3 | Status subresource manipulation via stale writes | N/A (race condition) | Medium | Low | Mitigated | +| T4 | Jinja2 template injection from CR spec fields | Cluster user (malicious CR) | High | Medium | Mitigated | +| T5 | Untrusted playbook code execution | Operator author | Critical | Low | Accepted | +| T6 | ansible-runner not on PATH | N/A (misconfiguration) | Low | Low | Mitigated | +| T7 | Resource exhaustion from concurrent reconciles | Cluster user (CR flood) | Medium | Medium | Mitigated | +| T8 | Proxy exposed outside pod | Network attacker | Critical | Low | Mitigated | +| T9 | Authorization header leakage to API server | N/A (code bug) | High | Low | Mitigated | +| T10 | Metrics API exposed externally | Network attacker | Low | Low | Mitigated | +| T11 | Event API socket access by other pods | Co-located pod | Medium | Low | Mitigated | +| T12 | HTTP/2 vulnerabilities | Network attacker | Medium | Low | Mitigated | +| T13 | Slowloris attacks | Network attacker | Low | Low | Mitigated | +| T14 | Kubeconfig temp files persisting on disk | Pod co-tenant | High | Low | Mitigated | +| T15 | Owner reference in Basic Auth username | N/A | Low | Low | Mitigated | +| T16 | Request logging exposing secrets | Operator author (verbose config) | Medium | Low | Documented | +| T17 | Vulnerable Python packages in operator image | Supply chain | High | Medium | Partially mitigated | +| T18 | Container escape | Container attacker | Critical | Low | Mitigated | +| T19 | Tampered testdata committed | Contributor (malicious PR) | Medium | Low | Mitigated | +| T20 | Stale vendor dependencies | Supply chain | Medium | Medium | Mitigated | + +### Mitigations + +| ID | Mitigation | +|---|---| +| T1 | Document that production operators should narrow permissions from scaffold default | +| T2 | 6-second cache timeout with fallback to API server | +| T3 | Status updates use `APIReader` (direct reads) to prevent stale writes | +| T4 | `markUnsafe: true` wraps strings as `__ansible_unsafe` | +| T5 | Operator runs playbooks defined by the operator author, not end users | +| T6 | Detected at reconcile time via `exec.LookPath` | +| T7 | `MaxConcurrentReconciles` bounded; configurable per-GVK | +| T8 | Binds to `localhost` only; kubeconfig hardcodes localhost URL | +| T9 | Two independent stripping points in handler chain | +| T10 | Binds to `localhost:5050` | +| T11 | Unix socket in `/tmp` with umask `0077` | +| T12 | HTTP/2 disabled by default (`--enable-http2=false`) | +| T13 | `ReadHeaderTimeout: 5s` on all HTTP servers | +| T14 | `defer os.Remove` in reconcile; cleanup runs even on panic | +| T15 | Base64-encoded metadata only, not credentials; stripped before API server | +| T16 | Verbose logging (`--log-requests`, V(2)) disabled by default | +| T17 | Pipfile.lock pins versions; Dependabot monitors image deps | +| T18 | Scaffolded pod security: `runAsNonRoot`, `seccompProfile: RuntimeDefault`, drops all capabilities | +| T19 | `make test-sanity` regenerates and asserts `git diff --exit-code` | +| T20 | CI dirty-tree check catches stale vendor | + +## 5. Deprioritized + +| Threat | Reason | +|---|---| +| Ansible sandbox escape | Operator author controls playbook content; sandboxing is out of scope for this project | +| SA token theft via `/proc` | Requires container escape (T18) which is separately mitigated | +| DNS rebinding against localhost proxy | Pod network policies and localhost binding make this impractical | +| Timing side-channels in reconcile loop | No security-sensitive branching in the reconcile path | + +## 6. Open questions + +- Formal security audit of the proxy handler chain ordering +- Review of downstream Cachito dependency resolution for supply chain risks +- Evaluation of ansible-runner sandboxing options +- Audit of file permissions in `/tmp/ansible-operator/runner/` (currently 0777 for directories) +- Whether projected SA token rotation is handled correctly during long-running reconciles + +## 7. Provenance + +- **Mode**: bootstrap +- **Date**: 2026-09-10 +- **Author**: AI-assisted draft, pending maintainer review +- **Methodology**: Manual code review of proxy, runner, and controller packages + +## 8. Recommended mitigations + +| Mitigation | Threat IDs | Effort | Priority | +|---|---|---|---| +| Narrow default scaffold RBAC to namespace-scoped | T1 | S | High | +| Add NetworkPolicy to scaffolded output restricting metrics ingress | T10 | S | Medium | +| Tighten `/tmp/ansible-operator/runner/` directory permissions to 0750 | T11, T14 | S | Medium | +| Add gosec to CI pipeline for SAST scanning | T17 | S | High | +| Document verbose logging security implications in AGENTS.md | T16 | S | Low | +| Evaluate ansible-runner process sandboxing (seccomp, AppArmor) | T5, T7 | L | Low | +| Add secret detection (gitleaks/detect-secrets) to pre-commit | T16 | M | Medium | diff --git a/docs/AOP_DEVELOPMENT.md b/docs/AOP_DEVELOPMENT.md new file mode 100644 index 00000000..fce117c4 --- /dev/null +++ b/docs/AOP_DEVELOPMENT.md @@ -0,0 +1,157 @@ +# Ansible Operator Plugins -- Development Guide + +## Quick Start + +### Prerequisites + +- Go 1.26.3 (CI reads the version from `go.mod`) +- Docker with buildx support (for image builds) +- make +- Ansible and ansible-runner (for E2E tests only) + +Tool binaries are managed via [bingo](https://github.com/bwplotka/bingo) -- +no manual tool installation needed. + +### Build and Run + +```bash +# Bootstrap dev tools (idempotent, no cluster creation) +make setup + +# Build the ansible-operator binary +make build + +# Full non-cluster validation (sanity + unit tests) +make verify + +# Lint only (no auto-fix) +make lint + +# Auto-fix: go mod tidy + go fmt + golangci-lint --fix +make fix + +# Regenerate testdata after scaffold template changes +make generate + +# Build Docker image +make image-build +``` + +## Validation Command Matrix + +| Change Type | Minimum Validation | Full PR Validation | +|---|---|---| +| Go package code | `go fmt ./path/to/pkg`, `go test ./path/to/pkg`, `go vet ./path/to/pkg` | `make verify` | +| Scaffold/plugin templates | `make generate` + `git diff --exit-code` | `make verify` | +| Go dependencies | `go mod tidy && go mod vendor` | `make verify` | +| Documentation only | Markdown link/render review | `make test-sanity` (for license/format if touching Go-adjacent files) | +| Downstream (openshift/) | `openshift/Makefile` targets | `check-requirements`, `check-collections` | + +## Single-File / Focused Change Validation + +Go is compiled and type-checked at the **package** level, not the file level. +You cannot validate a single `.go` file in isolation. Always work with the +package containing the file: + +```bash +# Format the file (applied to all files in the package) +go fmt ./internal/ansible/controller/ + +# Run a specific test +go test ./internal/ansible/controller/ -run TestReconcile + +# Vet the package +go vet ./internal/ansible/controller/ + +# Lint the package +golangci-lint run ./internal/ansible/controller/ +``` + +### When Repo-Wide Checks Are Mandatory + +- Scaffold template changes (`pkg/plugins/`) -- must run `make generate` +- `go.mod` changes -- must run `go mod tidy && go mod vendor` +- Import path changes in `internal/` or `pkg/` +- Anything touching `hack/generate/` + +### Ansible/Python Changes + +Molecule test artifacts (`__pycache__/`, `.pytest_cache/`) are gitignored. +E2E molecule tests require `ansible-core` installed: + +```bash +pip3 install ansible-core~=2.17.4 +make test-e2e-ansible-molecule +``` + +## Safe Validation Behavior + +- `make test-sanity` runs `generate` and `fix` **before** checks, then + asserts `git diff --exit-code`. This means it may modify files and then + fail if the working tree becomes dirty. +- E2E tests (`make test-e2e-ansible`) require Docker and Kind. They create a + cluster, build images, and take several minutes. +- Unit tests run with `-short` flag via `make test-unit`. Tests requiring a + live cluster are skipped in short mode. + +## Code Conventions + +### License Header + +Every `.go` file must have an Apache 2.0 license header. Enforced by +`hack/check-license.sh` during `make test-sanity`. + +### Error and Log Message Formatting + +Enforced by `hack/check-error-log-msg-format.sh`: +- Log messages (Error, Fatal, Info, Warn) must begin with an uppercase letter. +- Error messages (`errors.New`, `fmt.Errorf`) must begin with a lowercase letter. +- Error messages must not end with a period. + +### Import Ordering + +Three groups separated by blank lines: +1. Standard library +2. Third-party and Kubernetes libraries +3. Internal packages (`github.com/operator-framework/ansible-operator-plugins/...`) + +### Logging + +Use `logf "sigs.k8s.io/controller-runtime/pkg/log"` with `logf.Log.WithName("...")`. +Verbosity: V(0) for reconciliation lifecycle, V(1) for handler events, +V(2) for request bodies and status events. + +### Naming Conventions + +- Controllers: `--controller` (lowercased) +- Env var overrides: `__` (dots → underscores, uppercased) +- Annotations: prefix `ansible.sdk.operatorframework.io/` +- File permissions: `DirMode = 0755`, `FileMode = 0644`, `ExecFileMode = 0755` + +## Vendoring + +This project vendors all dependencies. After modifying `go.mod`: + +```bash +go mod tidy +go mod vendor +``` + +Commit the updated `vendor/`. The `make fix` target runs `go mod tidy` but +does not run `go mod vendor`. + +## Release Process + +Releases are tag-driven. See [docs/decisions/adr-0003-release-rebase-workflow.md](decisions/adr-0003-release-rebase-workflow.md) for the full workflow. + +1. Update `ImageVersion` in `internal/version/version.go` +2. Update `IMAGE_VERSION` in `Makefile` +3. Run `make generate` +4. Merge the prep PR, then tag + +## Downstream / OpenShift + +See [docs/references/downstream-sync.md](references/downstream-sync.md) for +the full rebase workflow, `UPSTREAM: :` convention, and downstream +Make targets. + diff --git a/docs/AOP_TESTING.md b/docs/AOP_TESTING.md new file mode 100644 index 00000000..110c928c --- /dev/null +++ b/docs/AOP_TESTING.md @@ -0,0 +1,98 @@ +# Ansible Operator Plugins -- Testing Guide + +## Test Frameworks and Assertion Libraries + +This repository uses two testing approaches side by side: + +- **Ginkgo/Gomega (BDD style):** Used for handler tests, flags, metrics, proxy, + version, and all E2E tests. Packages using Ginkgo dot-import both `ginkgo/v2` + and `gomega`. +- **Standard `testing` + `testify/assert`:** Used for controller reconcile tests, + watches, runner, paramconv, k8sutil, and status utilities. +- **Do not mix** Ginkgo and testify within the same package. + +## Suite Files + +Every Ginkgo package requires a `*_suite_test.go` file named +`_suite_test.go`. The suite file must define a `TestXxx(t *testing.T)` +function that calls `RegisterFailHandler(Fail)` then `RunSpecs`. + +## Short Mode and Test Skipping + +Unit tests run with `-short` flag via `make test-unit`. Tests requiring a live +cluster must guard with: + +- Standard tests: `if testing.Short() { t.Skip("...") }` +- Ginkgo specs: `if testing.Short() { Skip("...") }` inside the `It` block +- E2E tests are excluded from `test-unit` via package path filtering (`grep -v test/`) + +## Table-Driven Tests + +Standard-library tests use table-driven patterns with named test cases. +Always include a `Name` field. Use `ShouldError bool` for error expectation +fields, matching existing convention. + +## Fake Runner + +Use `internal/ansible/runner/fake.Runner` to stub the Ansible runner interface: + +- `JobEvents []eventapi.JobEvent` -- events returned from `Run()` +- `Error error` -- makes `Run()` return this error +- `Finalizer string` -- returned by `GetFinalizer()` +- `Stdout string` -- stdout content for the run result + +Do not create new runner mock implementations; use and extend this fake. + +## Fake Client (controller-runtime) + +Use `sigs.k8s.io/controller-runtime/pkg/client/fake` for unit-testing reconcilers. +When checking status subresource updates, register objects with `WithStatusSubresource`. + +## envtest + +The `handler` package uses `envtest.Environment` for integration tests. Start in +`BeforeSuite`, stop in `AfterSuite`. + +## Logging in Tests + +Handler tests capture log output to a shared `bytes.Buffer` set via +`zap.New(zap.WriteTo(&logBuffer), zap.UseDevMode(true))`. Reset before each +assertion. Verify content using `MatchRegexp`. + +## Test Data + +Static fixtures live in `testdata/` directories adjacent to test files: + +- `internal/ansible/watches/testdata/` -- YAML fixtures for watch loading +- `internal/ansible/runner/testdata/` -- playbooks and roles for runner tests +- `testdata/memcached-molecule-operator/` -- full sample operator for E2E + +Template-based fixtures are rendered at test time and cleaned up with `defer os.Remove`. + +## E2E Test Infrastructure + +### Test Utilities (`pkg/testutils/`) + +- `command.CommandContext` -- wraps exec with dir/env/stdin +- `kubernetes.Kubectl` -- kubectl command interface +- `sample.Sample` -- scaffolded operator project interface +- `e2e/operator` -- BuildOperatorImage, DeployOperator, UndeployOperator, InstallCRDs +- `e2e/kind` -- Kind cluster detection and image loading +- `e2e/prometheus` -- Prometheus operator install/uninstall +- `e2e/metrics` -- metrics scraping and verification + +### E2E Test Lifecycle + +1. `BeforeSuite`: generate sample project, configure kubectl, build and load operator image +2. `BeforeEach`: install CRDs, deploy operator +3. `It`: apply CRs, poll with `Eventually`, check logs/status/metrics +4. `AfterEach`: delete CRs, undeploy operator +5. `AfterSuite`: uninstall Prometheus, remove docker image, remove test directory + +## Key Conventions + +1. In Ginkgo tests, structure as `Describe` > `Context`/`When` > `It`. Use `BeforeEach` for per-spec setup. +2. Use `By("description")` in Ginkgo specs for test step documentation. +3. Unstructured objects in reconciler tests must include `apiVersion`, `kind`, and `metadata`. +4. Clean up env var changes with `defer os.Unsetenv` or `t.Setenv`. +5. E2E tests check `api-resources` for `servicemonitors` to avoid duplicate Prometheus installs. diff --git a/docs/architecture/boundaries.md b/docs/architecture/boundaries.md new file mode 100644 index 00000000..a74d9f33 --- /dev/null +++ b/docs/architecture/boundaries.md @@ -0,0 +1,40 @@ +# Architectural Boundaries + +## Package Visibility and Dependency Direction + +```text +cmd/ Binary entrypoint only; delegates to internal/cmd/ + └─→ internal/ Runtime implementation (not importable by external consumers) + └─→ pkg/ Public API surface (kubebuilder plugin, test utilities) + +openshift/ Independent build overlay; mirrors upstream, does not + import upstream Go packages at build time + +vendor/ Committed dependency tree; leaf artifact, never hand-edited +testdata/ Generated sample projects; leaf artifact, never hand-edited +``` + +### Allowed Import Directions + +| Source | May Import | +|---|---| +| `cmd/` | `internal/`, `pkg/`, stdlib, third-party | +| `internal/` | `pkg/`, stdlib, third-party, vendored | +| `pkg/` | stdlib, third-party, vendored | +| `openshift/` | Its own `go.mod` dependencies (separate module) | + +### Forbidden Patterns + +- `pkg/` must never import `internal/`. The plugin scaffolding package is a + public API; it must not depend on runtime internals. +- `cmd/` should not contain business logic beyond CLI wiring. +- `openshift/` Go code does not import the root module's packages at build time + (it has its own `go.mod`). + +## Proxy Handler Chain Ordering + +The handler chain in `proxy.go` is assembled inside-out and the ordering is +load-bearing. See `docs/domain/watches-and-contracts.md` for the contract. + +Adding middleware that modifies request bodies must go between the +authorization-stripping and owner-injection layers, never outside the cache handler. diff --git a/docs/architecture/components.md b/docs/architecture/components.md new file mode 100644 index 00000000..2b275707 --- /dev/null +++ b/docs/architecture/components.md @@ -0,0 +1,98 @@ +# Architecture: Components and Integration + +## Architecture Overview + +This project bridges Ansible automation with Kubernetes controllers via +controller-runtime. The main loop: a `watches.yaml` file maps GVKs to Ansible +playbooks/roles. For each watch entry, a controller-runtime controller is +created that, on reconcile, shells out to `ansible-runner`, consumes job events +via a Unix-domain-socket HTTP API, and updates the CR's status conditions. + +## Watches Configuration (internal/ansible/watches/) + +1. Every Watch must specify exactly one of `playbook` or `role`, never both. +2. Default values: `manageStatus: true`, `watchDependentResources: true`, + `watchClusterScopedResources: false`, `snakeCaseParameters: true`, + `maxRunnerArtifacts: 20`, `ansibleVerbosity: 2`. +3. Watches supports `${VAR}` environment variable interpolation via `os.Expand`. +4. Per-GVK concurrency: `MAX_CONCURRENT_RECONCILES__` (dots → underscores, uppercased). Legacy `WORKER__` still works. +5. Role paths support FQCN format resolved against `ANSIBLE_COLLECTIONS_PATH`. +6. The `selector` field produces a `LabelSelectorPredicate` that filters which CRs trigger reconciliation. + +## Controller-Runtime Integration (internal/ansible/controller/) + +7. Controllers are named `--controller` (lowercased), registered via `controller.New`. +8. Unregistered GVKs are dynamically added to the scheme as `unstructured.Unstructured`. +9. Default predicate stack: `GenerationChangedPredicate` OR `NoGenerationPredicate`. If `watchAnnotationsChanges` is true, `AnnotationChangedPredicate` is OR'd in. +10. The primary watch uses `LoggingEnqueueRequestForObject` for metrics + logging. +11. `controller.Add` returns a `*controller.Controller` pointer stored in the `ControllerMap` so the proxy can dynamically add dependent watches. + +## Reconciliation Loop (internal/ansible/controller/reconcile.go) + +12. `APIReader` (direct API reads, bypassing cache) is used in two places: `markRunning`/`markError`/`markDone` call it to refresh the object immediately before `Client.Status().Update`, and after `Runner.Run` completes the reconciler calls it again to re-read the CR from the API server (ansible may have modified it via the proxy during the run). +13. Per-CR reconcile period override: annotate with `ansible.sdk.operatorframework.io/reconcile-period: `. +14. Finalizer lifecycle: added on first reconcile if configured, removed only after a successful finalizer run on deletion. +15. If the CR has no `spec`, an empty map is injected so ansible parameters work for Secrets/ConfigMaps. +16. The `requeue_after` module in ansible overrides `RequeueAfter` when detected from event data. +17. Failed tasks (`runner_on_failed`) that are neither `IgnoreError()` nor `Rescued()` produce failure messages. +18. A `playbook_on_stats` event must be present somewhere in the event stream (it is recorded as it is seen, then checked after the stream closes). If missing, reconciliation fails. + +## Status Management (internal/ansible/controller/status/) + +19. Three condition types: `Running`, `Failure`, `Successful`. When `manageStatus` is true, the reconciler transitions between these. +20. `SetCondition` is a no-op when `Type`, `Status`, and `Reason` are unchanged, except for `FailureConditionType` which always updates. +21. Status is serialized via `GetJSONMap()` (marshal/unmarshal cycle) because `unstructured.Unstructured` has special DeepCopy rules. + +## Ansible Runner Integration (internal/ansible/runner/) + +22. The `Runner` interface: `Run(ident, *unstructured.Unstructured, kubeconfig) (RunResult, error)` and `GetFinalizer() (string, bool)`. +23. `ansible-runner` must be on `$PATH`. Detected via `exec.LookPath` at run time, not startup. +24. Input directory layout: `/tmp/ansible-operator/runner//////` with `env/`, `project/`, `inventory/`. +25. Parameters in `env/extravars`: snake_cased spec, `ansible_operator_meta`, `__`, watch vars, finalizer vars. +26. The `markUnsafe` feature wraps all string values as `{"__ansible_unsafe": ""}`. +27. After each run, a `latest` symlink is created under `artifacts/`. + +## Event API (internal/ansible/runner/eventapi/) + +28. Each reconcile creates a Unix socket at `/tmp/ansibleoperator-` and starts an HTTP server. +29. The events channel is buffered with capacity 1000. A 10-second write timeout prevents blocking. +30. Status events (those without a UUID) are silently dropped; only JobEvents with a UUID are forwarded. + +## Proxy Server (internal/ansible/proxy/) + +31. Runs on localhost (default port 8888). Ansible connects via a generated kubeconfig. +32. Owner reference injection on POST; falls back to annotation-based ownership for cross-namespace/cluster-scoped resources. +33. Cache response handler serves GETs from informer cache (`X-Cache: HIT`). Cache timeout is 6 seconds. +34. Dependent watch registration on resource creation. +35. The `blacklist` field in watches.yaml prevents specific GVKs from being watched or cached. + +## Event Handler Wrappers (internal/ansible/handler/) + +36. Three handler wrappers, all adding structured logging at V(1): + - `LoggingEnqueueRequestForObject`: primary resource watches + - `LoggingEnqueueRequestForAnnotation`: annotation-based dependent watches + - `EnqueueRequestForOwnerWithLogging`: owner-based dependent watches (full reimplementation) + +## Prometheus Metrics (internal/ansible/metrics/) + +37. Built-in metrics (subsystem `ansible_operator`): `reconcile_result` (Gauge), `reconciles` (Histogram), `build_info` (Gauge). +38. Registered with controller-runtime's `metrics.Registry`, not the default prometheus registry. +39. User-defined metrics via the API server on port 5050. Type changes are rejected after initial registration. +40. All metric operations wrapped with `recover()` to prevent operator crashes. + +## Kubeconfig Generation (internal/ansible/proxy/kubeconfig/) + +41. A temporary kubeconfig per reconcile, pointing to the proxy (`http://localhost:8888`). Owner reference base64-encoded into Basic Auth username. +42. Uses `insecure-skip-tls-verify: true` for the local proxy connection. + +## ControllerMap (internal/ansible/proxy/controllermap/) + +43. Thread-safe `map[GVK]*Contents` bridging controllers and the proxy. All access is mutex-protected. +44. `WatchMap` tracks which dependent GVKs already have watches registered, preventing duplicates. + +## Plugin and Scaffolding (pkg/plugins/ansible/v1/) + +45. Implements the kubebuilder plugin interface for scaffolding Ansible-based operators. +46. Templates in `scaffolds/internal/templates/` generate: Dockerfile, watches.yaml, molecule tests, RBAC manifests, kustomize config. +47. Scaffold output goes to `testdata/` via `make generate`. Edit templates, never testdata directly. + diff --git a/docs/architecture/error-handling.md b/docs/architecture/error-handling.md new file mode 100644 index 00000000..c3b4b595 --- /dev/null +++ b/docs/architecture/error-handling.md @@ -0,0 +1,129 @@ +# Error Handling + +## Reconciler Error Contract + +The `Reconcile` method returns `(reconcile.Result, error)` to controller-runtime. + +1. **Not-found on initial Get is not an error.** Return `(Result{}, nil)` when + `apierrors.IsNotFound(err)` on the primary resource fetch. + +2. **The return value is branch-specific, not one universal rule.** Which + result/error pair is returned depends on where in `Reconcile` the error + originates: + + | Branch | Returns | + |---|---| + | Initial `Client.Get` — not found | `reconcile.Result{}, nil` | + | Initial `Client.Get` — other error | `reconcile.Result{}, err` | + | Reconcile-period annotation parse, finalizer update, kubeconfig, or `Runner.Run` errors | `reconcileResult, err` (after attempting `markError`) | + | `markRunning` failure (`ManageStatus=true`) | `reconcileResult, errmark` | + | Event JSON marshal/unmarshal error | `reconcile.Result{}, err` (bypasses `reconcileResult`) | + | Post-run `APIReader.Get` — not found | `reconcile.Result{}, nil` | + | Post-run `APIReader.Get` — other error | `reconcile.Result{}, err` | + | Missing `playbook_on_stats` event | `reconcileResult, errors.New("did not receive playbook_on_stats event")` | + | `markDone` (`ManageStatus=true`), task failures present | `reconcileResult, errors.New("event runner on failed")` | + | `markDone` (`ManageStatus=true`), success | `reconcileResult, errmark` (the status-update error, if any) | + | `ManageStatus=false`, task failures present | `reconcileResult, errors.New("received failed task event")` | + + Only the branches that call `markError`/`markRunning`/`markDone` use the + pre-computed `reconcileResult` (with `RequeueAfter`); the initial and + post-run API reads intentionally return a bare `reconcile.Result{}`. + +3. **Ansible run failures produce `errors.New` sentinel strings, not wrapped + errors.** Two distinct messages: + - `"event runner on failed"` (ManageStatus=true) + - `"received failed task event"` (ManageStatus=false) + + These are intentionally flat strings. Do not change them to `fmt.Errorf` with `%w`. + +4. **Missing `playbook_on_stats` is a reconciliation error.** If the event + stream ends without a stats event, return + `errors.New("did not receive playbook_on_stats event")`. + +## Status Marking Pattern + +Every reconciler error must attempt a corresponding status update before +returning. The pattern uses paired variables (`err` / `errmark`): + +Rules: +- Always return the **original** error to controller-runtime, never the status-update error. +- Log the status-update error separately if it fails. +- `markError` calls `metrics.ReconcileFailed` immediately, so metrics are recorded even if the status update fails. +- In `markError` and `markDone`, treat `apierrors.IsNotFound` as a no-op (resource was deleted). +- **Exception:** `markRunning` does not follow the `err`/`errmark` pairing above. + There is no separate "original" error to preserve at that point in the flow, + so its own return value (`errmark`) is returned directly to controller-runtime + on failure. Similarly, on the successful path through `markDone` + (`ManageStatus=true`, no task failures), `errmark` is also returned directly + since there is no prior error to prioritize over it. + +## Metric Panic Recovery + +`recoverMetricPanic()` uses `defer` to catch panics in Prometheus metric operations. +All exported metric functions (`ReconcileSucceeded`, `ReconcileFailed`, +`ReconcileTimer`) must use `defer recoverMetricPanic()` as their first statement. + +## Error Wrapping + +This codebase uses `%w` wrapping sparingly: +- `watches.go`: `fmt.Errorf("invalid GVK: %s: %w", gvk, err)` +- `internal/util/k8sutil/api.go`: wraps file I/O errors + +When adding new errors: +- Use `%w` when the caller needs `errors.Is` or `errors.As`. +- Use `errors.New` or `%v` for human-readable messages that will only be logged. + +## Kubernetes API Error Handling + +Use the `apierrors` package for API server errors. The only type check in the +codebase is `apierrors.IsNotFound`. Use `runtime.IsNotRegisteredError` for +scheme registration checks. + +## HTTP Handler Error Patterns + +### Proxy Handlers + +Two strategies depending on severity: + +1. **Return an HTTP error and stop:** When failure would cause incorrect behavior + (e.g., missing owner reference prevents garbage collection): + ```go + log.Error(err, m) + http.Error(w, m, http.StatusInternalServerError) + return + ``` + +2. **Log and fall through to API server:** When the cache might not have data: + ```go + log.Error(err, "Cache miss, can not find in rest mapper") + break // falls through to c.next.ServeHTTP(w, req) + ``` + +The `break` vs. `return` distinction is critical. + +### Event API Handler + +HTTP status codes: `404` (path not found), `405` (wrong method), `415` (wrong content type), +`400` (bad JSON), `410 Gone` (receiver stopped), `500` (read failure or channel timeout), +`204 No Content` (success). + +### Metrics API Server + +Metric validation errors return `400 Bad Request`. Use `log.Info(err.Error())` +(not `log.Error`) for client-caused errors. + +## Runner Error Handling + +- Missing `ansible-runner` binary: return `exec.LookPath` error directly. +- Async goroutine errors are logged but not returned (the `Run` method has already returned). +- `http.ErrServerClosed` from the event API is explicitly ignored as a clean shutdown signal. +- Use `errors.Is(err, os.ErrNotExist)` for artifact symlink checks. + +## Status Condition Types + +Three condition types track reconciliation state: +- `Running` -- set to True when reconciliation starts +- `Failure` -- set to True on error +- `Successful` -- set to True on successful completion + +When one becomes True, the others are set to False. diff --git a/docs/architecture/performance.md b/docs/architecture/performance.md new file mode 100644 index 00000000..d231a9d8 --- /dev/null +++ b/docs/architecture/performance.md @@ -0,0 +1,76 @@ +# Performance + +Rules and conventions for concurrency, resource management, and throughput. + +## 1. Event Channel Buffering + +The event API channel is created with a fixed buffer of 1000 in +`internal/ansible/runner/eventapi/eventapi.go`. Do not change this buffer +size without load-testing. Every write uses a `select` with a 10-second +`time.NewTimer` timeout. Always stop timers after use. + +## 2. EventReceiver Lifecycle + +`EventReceiver.Close()` performs: sets `stopped = true` under a write lock, +closes the HTTP server, removes the unix socket, and closes the Events channel. + +Any new handler paths that touch the Events channel must acquire RLock and +check `stopped` first. `Close()` must always be called after ansible-runner exits. + +## 3. MaxConcurrentReconciles + +Default is `runtime.NumCPU()`. Each concurrent reconcile spawns an +ansible-runner subprocess, a unix-socket HTTP server, and a goroutine. +Budget roughly `3 * MaxConcurrentReconciles` goroutines plus one subprocess each. + +Per-GVK override via `MAX_CONCURRENT_RECONCILES__` takes +precedence over the CLI flag. Values <= 0 are replaced with the default. + +## 4. Cache Establishment Timeout + +Hardcoded as `cacheEstablishmentTimeout = 6 * time.Second` in `proxy.go`. +Applies to every `informerCache.Get` and `informerCache.List` call. Not +configurable at runtime. Increasing it delays fallback to the API server. + +## 5. RWMutex Locking Patterns + +Three distinct usages: + +### ControllerMap and WatchMap (`controllermap.go`) +- `sync.RWMutex` embedded in struct. `Get` takes RLock; `Store`/`Delete` take full Lock. +- Read on every proxied GET and owner-ref POST. Keep critical sections minimal. + +### apiResources (`proxy.go`) +- `*sync.RWMutex` (pointer). Lock-upgrade pattern: RLock for cache hit, full Lock + for discovery refresh via `ServerGroupsAndResources`. Cache miss blocks all concurrent checks. + +### EventReceiver (`eventapi.go`) +- Protects only the `stopped` boolean. Channel is closed only when `stopped == true`. + +**General rule:** prefer `sync.RWMutex`. Do not use `sync.Map`. + +## 6. Goroutine Launch Patterns + +| Location | Purpose | Lifecycle | +|---|---|---| +| `runner.go` Run() | Runs ansible-runner subprocess, closes receiver | Until subprocess exits | +| `eventapi.go` New() | Serves event HTTP on unix socket | Until `receiver.Close()` | +| `proxy.go` Run() | Starts informer cache; serves proxy | Process lifetime | +| `reconcile.go` Reconcile() | Fire-and-forget event handler dispatch | No join | +| `cache_response.go` | Recovers dependent watches | No join | + +Only the runner goroutine cleans up the EventReceiver. Event handlers must be +goroutine-safe and must not modify the `unstructured.Unstructured` object. + +## 7. Resource Cleanup + +- **Unix sockets**: Created at `/tmp/ansibleoperator-`, removed in `EventReceiver.Close()`. +- **Kubeconfig temp files**: Created per reconcile, removed via `defer os.Remove(kc.Name())`. +- **Runner artifacts**: Controlled by `maxRunnerArtifacts` (default 20). A `latest` symlink is maintained. + +## 8. Error Channel Pattern + +The runner creates `make(chan error, 1)`. The buffer size of 1 prevents the +HTTP server's `Serve` goroutine from blocking. Use this same pattern for any +new background error communication. + diff --git a/docs/decisions/adr-0001-upstream-downstream-mirror.md b/docs/decisions/adr-0001-upstream-downstream-mirror.md new file mode 100644 index 00000000..1e627440 --- /dev/null +++ b/docs/decisions/adr-0001-upstream-downstream-mirror.md @@ -0,0 +1,44 @@ +# ADR-0001: Upstream/Downstream Mirror Architecture + +## Status + +Accepted + +## Context + +This repository (`openshift/ansible-operator-plugins`) is the downstream mirror +of the upstream Ansible operator source at +`operator-framework/ansible-operator-plugins`. The root tree mirrors upstream +unmodified; the `openshift/` directory adds an independent build overlay, +downstream-only dependencies, and Ansible collections. The two trees must stay +synchronized without polluting each other's histories. + +## Decision + +The `openshift/` directory at the root of this repository contains the +downstream overlay as a self-contained subtree with its own `go.mod`, +`vendor/`, `Makefile`, and `Dockerfile`. This allows: + +1. The root tree to remain a clean mirror of upstream, free of OpenShift-specific build concerns. +2. Downstream maintainers to rebase from upstream tags using + `openshift/hack/rebase_upstream.sh`. +3. Commits introduced during rebase to use the `UPSTREAM: :` + prefix convention to signal intent during future rebases. + +The downstream overlay does **not** import upstream Go packages at build time. +It has its own module path and dependency tree. + +We chose to keep the overlay as a separate module tree instead of a shared Go +workspace because downstream CVE backports and release cadence must not +block on upstream merges (or vice versa). A shared workspace would tie both +trees to the same dependency graph and release timeline. + +## Consequences + +- Contributors must understand which directory to modify based on whether a + change is upstream (root) or downstream-only (`openshift/`). +- Dependency updates may need to happen in two places (`go.mod` and `openshift/go.mod`). +- The `UPSTREAM: :` / `UPSTREAM: :` convention must be followed + during downstream rebases to preserve the correct commit history. +- See [docs/references/downstream-sync.md](../references/downstream-sync.md) + for the full rebase workflow. diff --git a/docs/decisions/adr-0002-generated-vendor-artifact-policy.md b/docs/decisions/adr-0002-generated-vendor-artifact-policy.md new file mode 100644 index 00000000..a1ac6c45 --- /dev/null +++ b/docs/decisions/adr-0002-generated-vendor-artifact-policy.md @@ -0,0 +1,37 @@ +# ADR-0002: Generated and Vendor Artifact Policy + +## Status + +Accepted + +## Context + +The repository contains multiple categories of generated artifacts: `testdata/` +(scaffold sample projects), `vendor/` (Go dependencies), and downstream +collections/requirements under `openshift/`. CI enforces a clean working tree +after generation and formatting via `git diff --exit-code`. + +## Decision + +1. **`vendor/`** is committed to the repository. All Go dependency changes + require `go mod tidy && go mod vendor` followed by committing the vendor + directory. + +2. **`testdata/`** is entirely machine-generated from scaffold templates. + It is regenerated by `make generate` and must never be hand-edited. + +3. **Downstream artifacts** (`openshift/release/ansible/ansible_collections/`, + `openshift/vendor/`) are generated during downstream rebases and maintained + via `openshift/Makefile` targets. + +4. **CI enforcement**: `make test-sanity` runs `generate` and `fix` first, + then asserts `git diff --exit-code`. This catches any stale generated + artifacts as a PR failure. + +## Consequences + +- PRs that modify scaffold templates must include regenerated testdata. +- The `vendor/` directory is large but enables reproducible builds without + network access. +- Contributors must not hand-edit generated artifacts; the correct workflow is + to modify the source (templates, `go.mod`) and regenerate. diff --git a/docs/decisions/adr-0003-release-rebase-workflow.md b/docs/decisions/adr-0003-release-rebase-workflow.md new file mode 100644 index 00000000..c540c03f --- /dev/null +++ b/docs/decisions/adr-0003-release-rebase-workflow.md @@ -0,0 +1,39 @@ +# ADR-0003: Release and Rebase Workflow + +## Status + +Accepted + +## Context + +Releases are tag-driven and automated via goreleaser. The downstream OpenShift +fork periodically rebases onto upstream release tags, introducing +downstream-specific commits that must be tracked across rebases. + +## Decision + +### Upstream Release + +1. Update `ImageVersion` in `internal/version/version.go` and `IMAGE_VERSION` + in `Makefile`. +2. Run `make generate` to regenerate testdata. +3. Merge the release prep PR, then tag (e.g., `v1.42.3`). +4. The goreleaser GitHub Actions workflow builds multi-arch binaries and Docker + images, pushing to `quay.io/operator-framework/ansible-operator`. + +### Downstream Rebase + +1. Fetch the new upstream tag. +2. Run `openshift/hack/rebase_upstream.sh` which: + - Rebases the downstream branch onto the upstream tag + - Runs `go mod tidy && go mod vendor` with an `UPSTREAM: :` commit + - Updates ansible collections with an `UPSTREAM: :` commit + - Updates downstream requirements with an `UPSTREAM: :` commit +3. Resolve any Cachito conflicts manually. +4. Push the rebased branch for CI validation. + +## Consequences + +- The release process is simple for upstream: prep PR → tag → automated build. +- Downstream rebases require manual intervention for conflict resolution. +- The `UPSTREAM:` commit prefix convention must be preserved for future rebases. diff --git a/docs/decisions/adr-0004-openapi-not-applicable.md b/docs/decisions/adr-0004-openapi-not-applicable.md new file mode 100644 index 00000000..c9cd7762 --- /dev/null +++ b/docs/decisions/adr-0004-openapi-not-applicable.md @@ -0,0 +1,42 @@ +# ADR-0004: OpenAPI Not Applicable + +## Status + +Accepted — no OpenAPI artifact + +## Context + +An agentic readiness audit flagged the absence of OpenAPI/Swagger specifications +as a gap. The repository contains three internal HTTP servers: + +1. **REST proxy** (`internal/ansible/proxy/`) -- intercepts Ansible's K8s API calls +2. **Metrics API** (`internal/ansible/apiserver/`) -- accepts user-defined metrics on `localhost:5050` +3. **Event API** (`internal/ansible/runner/eventapi/`) -- receives ansible-runner events via Unix socket + +None of these are public-facing HTTP APIs with authored request/response schemas +suitable for OpenAPI documentation: + +- The **proxy** is a pass-through reverse proxy over the full Kubernetes API + surface -- it has no authored schema of its own to document; the schema is + Kubernetes' own OpenAPI, already published upstream. +- The **metrics API** accepts a simple JSON struct (`internal/ansible/metrics.UserMetric`) + on localhost only; its contract is documented in prose in + [docs/domain/watches-and-contracts.md](../domain/watches-and-contracts.md) + and in the Go source (`internal/ansible/metrics/metrics.go`). +- The **event API** communicates over a Unix domain socket using + `ansible-runner`'s internal event-stream format, not a request/response + HTTP contract that OpenAPI models well. + +## Decision + +No `openapi.yaml` is maintained in this repository. The `openapi_specs` attribute +is excluded from AgentReady assessment via `.agentready-config.yaml`. + +API contracts are documented in prose in +[docs/domain/watches-and-contracts.md](../domain/watches-and-contracts.md) and +in the Go type definitions themselves. + +## Consequences + +- No OpenAPI spec to maintain or keep in sync with source code. +- If a public API surface is ever added, this decision should be revisited. diff --git a/docs/decisions/adr-template.md b/docs/decisions/adr-template.md new file mode 100644 index 00000000..05c331c0 --- /dev/null +++ b/docs/decisions/adr-template.md @@ -0,0 +1,17 @@ +# ADR-NNNN: Title + +## Status + +Proposed | Accepted | Deprecated | Superseded by [ADR-NNNN](adr-nnnn-title.md) + +## Context + +What is the issue that we're seeing that is motivating this decision or change? + +## Decision + +What is the change that we're proposing and/or doing? + +## Consequences + +What becomes easier or more difficult to do because of this change? diff --git a/docs/domain/generated-artifacts.md b/docs/domain/generated-artifacts.md new file mode 100644 index 00000000..d5fcbc40 --- /dev/null +++ b/docs/domain/generated-artifacts.md @@ -0,0 +1,40 @@ +# Generated and Vendored Artifacts + +This document lists all generated artifacts in the repository, their regeneration +commands, and rules for when they must be updated. + +## Artifact Inventory + +| Artifact | Location | Regeneration Command | Hand-Edit? | +|---|---|---|---| +| Sample operator projects | `testdata/` | `make generate` | Never | +| Go dependencies | `vendor/` | `go mod tidy && go mod vendor` | Never | +| Scaffold templates output | `testdata/` (via templates in `pkg/plugins/`) | `make generate` | Never (edit templates instead) | +| Downstream ansible collections | `openshift/release/ansible/ansible_collections/` | `openshift/hack/rebase_upstream.sh` | Never | +| Downstream vendor | `openshift/vendor/` | `cd openshift && go mod tidy && go mod vendor` | Never | +| Downstream requirements | `openshift/release/ansible/requirements.yml` | `openshift/Makefile` targets | Never | + +## Rules + +1. **`testdata/`** is entirely generated. After any change to scaffolding + templates in `pkg/plugins/ansible/v1/scaffolds/`, run `make generate` and + commit the updated testdata. The `make test-sanity` target will fail + (`git diff --exit-code`) if testdata is stale. + +2. **`vendor/`** is committed. After modifying `go.mod`, always run: + ```sh + go mod tidy + go mod vendor + ``` + The `make fix` target runs `go mod tidy` but does **not** run + `go mod vendor`. The sanity check catches dirty vendor via `git diff`. + +3. **Scaffold templates** live in `pkg/plugins/ansible/v1/scaffolds/internal/templates/`. + Edit templates, not their output in `testdata/`. After template changes, + `make generate` rebuilds the binary and re-runs + `hack/generate/samples/generate_testdata.go`. + +4. **Downstream artifacts** under `openshift/` have their own generation + pipeline. See [docs/references/downstream-sync.md](../references/downstream-sync.md) + for the rebase workflow and `UPSTREAM: :` commit convention. + diff --git a/docs/domain/watches-and-contracts.md b/docs/domain/watches-and-contracts.md new file mode 100644 index 00000000..bf55c63a --- /dev/null +++ b/docs/domain/watches-and-contracts.md @@ -0,0 +1,100 @@ +# Watches and API Contracts + +## watches.yaml Contract + +Each entry requires `group`, `version`, `kind`, and exactly one of `playbook` or `role`. Version and Kind are mandatory; Group may be empty. + +- Duplicate GVKs in the same file cause a load error. +- Environment variables are expanded using `${VAR}` syntax. Undefined variables are left as literal `${VAR}`. +- Defaults: `manageStatus: true`, `watchDependentResources: true`, `watchClusterScopedResources: false`, `snakeCaseParameters: true`, `maxRunnerArtifacts: 20`, `reconcilePeriod: 0s`, `watchAnnotationsChanges: false`, `markUnsafe: false`. + +### Per-GVK Environment Variable Overrides + +- `MAX_CONCURRENT_RECONCILES_{KIND}_{GROUP}` overrides max concurrent reconciles (dots replaced with underscores, uppercased). +- `WORKER_{KIND}_{GROUP}` is the deprecated equivalent. If both are set, `MAX_CONCURRENT_RECONCILES_*` wins. +- `ANSIBLE_VERBOSITY_{KIND}_{GROUP}` overrides verbosity (valid range: 0-7). + +### Finalizer Contract + +- A finalizer must have a non-empty `name` field. +- It must specify a `role`, `playbook`, or non-empty `vars`. If none are provided, validation fails. +- If only `vars` is set (no role/playbook), the main watch's role/playbook is used as the finalizer command. + +### Role Path Resolution + +Role paths support FQCN format (`namespace.collection.role`), resolved against `ANSIBLE_COLLECTIONS_PATH` or default paths (`/usr/share/ansible/collections`, `~/.ansible/collections`). + +The `selector` field in watches.yaml produces a `LabelSelectorPredicate` that filters which CRs trigger reconciliation. + +## Metrics API Server (internal/ansible/apiserver) + +- Runs on `localhost:5050` and exposes a single endpoint: `POST /metrics`. +- Only POST is accepted; all other methods return `405 Method Not Allowed`. +- Request body must be JSON-decodable into a `metrics.UserMetric` struct. Malformed JSON returns `400 Bad Request`. +- A valid request must contain exactly one metric type (`counter`, `gauge`, `histogram`, or `summary`). Zero or more than one returns 400. +- Metrics are auto-registered with controller-runtime's Prometheus registry on first use. Once a metric name is registered with a type, that type is immutable. + +### UserMetric JSON Field Mapping + +The JSON field name for the metric description is `description`, not `help`: +```json +{"name": "my_metric", "description": "A help string", "counter": {"increment": true}} +``` + +## REST Proxy Server (internal/ansible/proxy) + +### Handler Chain Ordering + +The proxy applies HTTP handler middleware in a strict order: +1. **cacheResponseHandler** (outermost for GET) -- intercepts GETs and serves from informer cache when possible +2. **injectOwnerReferenceHandler** -- intercepts POSTs and adds ownerReferences or owner annotations +3. **removeAuthorizationHeader** -- strips the Authorization header so the proxy can re-inject its own +4. **RequestLogHandler** (optional) -- logs request method, URI, and body +5. **reverse proxy to real API server** (innermost) + +### Cache Response Contract + +- Only GET requests are candidates for cache lookup. All other methods pass through. +- Cache responses set `Content-Type: application/json` and `X-Cache: HIT`. +- Cache lookup is skipped for: subresources other than `status`, virtual resources, blacklisted GVKs, namespaces not in the watched set, and paths matching `AutoSkipCacheREList` (exec, attach). +- Cache operations use a 6-second context timeout. If the informer cache does not respond in time, the request falls through to the real API server. + +### Owner Reference Injection Contract + +- Owner references are only injected on POST (create) requests, never on subresource POSTs. +- The owner identity is extracted from the HTTP Basic Auth username field, which is a base64-encoded JSON `NamespacedOwnerReference`. +- When cross-namespace or cross-scope ownership prevents a native `ownerReference`, the proxy falls back to operator-lib owner annotations. +- After injecting, a dependent watch is registered on the controller so that changes to the created resource trigger owner reconciliation. + +## Event API (internal/ansible/runner/eventapi) + +### Unix Socket Protocol + +- Each ansible-runner invocation gets its own `EventReceiver` listening on `/tmp/ansibleoperator-{ident}`. +- The receiver accepts POST requests at `/events/` only. Non-POST returns `405`; wrong path returns `404`. +- Request Content-Type must be `application/json`. Otherwise returns `415`. +- Malformed JSON body returns `400`. Server errors return `500`. +- Successful event receipt returns `204 No Content`. +- After the receiver is stopped, further POSTs return `410 Gone`. + +### Event Filtering + +- Events without a UUID are status events from ansible-runner and are silently dropped. +- Only events with a non-empty `uuid` field are forwarded to the `Events` channel. +- The channel is buffered with capacity 1000. If the channel blocks for more than 10 seconds, the handler returns `500`. + +## Status Condition Types + +Three condition types are managed on the CR's `.status.conditions`: +- `Running` -- set to True when reconciliation starts +- `Failure` -- set to True on ansible failure or validation errors +- `Successful` -- set to True on successful reconciliation + +All status updates use the status subresource (`client.Status().Update()`). + +## Annotations Contract + +- `ansible.sdk.operatorframework.io/reconcile-period` -- overrides the controller's reconcile period for a specific CR. +- `ansible.sdk.operatorframework.io/max-runner-artifacts` -- overrides max artifacts per CR. +- `ansible.sdk.operatorframework.io/verbosity` -- overrides ansible verbosity per CR. + diff --git a/docs/patterns/README.md b/docs/patterns/README.md new file mode 100644 index 00000000..89319a16 --- /dev/null +++ b/docs/patterns/README.md @@ -0,0 +1,16 @@ +# Pattern Index + +Index of copy-modify reference implementations for the most common change +types in this repository. Each entry links to a real file (or a `.claude/skills/` +recipe) instead of describing the change in the abstract. + +| Change type | Reference implementation | +|---|---| +| New GVK watch entry | `internal/ansible/watches/watches.go`, `testdata/memcached-molecule-operator/watches.yaml`, `.claude/skills/add-watch-entry/SKILL.md` | +| Controller reconcile feature | `internal/ansible/controller/reconcile.go`, `.claude/skills/add-controller-feature/SKILL.md` | +| Scaffold template change | `pkg/plugins/ansible/v1/scaffolds/internal/templates/`, `.claude/skills/scaffold-template/SKILL.md` | +| Go dependency update | `go.mod`, `go.sum`, `.claude/skills/update-dependencies/SKILL.md` | +| Downstream carry patch | `openshift/Makefile`, `openshift/hack/rebase_upstream.sh`, `.claude/skills/downstream-carry/SKILL.md` | + +See also `examples/` for a runnable sample operator and `AGENTS.md` for the +full agent-facing documentation map. diff --git a/docs/references/downstream-sync.md b/docs/references/downstream-sync.md new file mode 100644 index 00000000..6e188a48 --- /dev/null +++ b/docs/references/downstream-sync.md @@ -0,0 +1,34 @@ +# Downstream Synchronization + +## Overview + +The `openshift/` directory contains an independent build overlay for the +OpenShift downstream fork. It has its own `go.mod`, `vendor/`, `Makefile`, +and `Dockerfile`. Changes to upstream code may require corresponding updates +in this overlay. + +For the full human walkthrough of the rebase process, see +[openshift/README.md](../../openshift/README.md). + +## UPSTREAM Commit Convention + +During downstream rebases, commits use a prefix convention to signal intent: + +| Prefix | Meaning | Example | +|---|---|---| +| `UPSTREAM: :` | Accept upstream version; discard downstream delta on next rebase | Vendor directory refresh | +| `UPSTREAM: :` | Preserve downstream-specific generated artifact across rebases | ansible_collections update, Cachito requirements | + +These prefixes are used by `openshift/hack/rebase_upstream.sh` and are +meaningful during future rebases. Do not add a stock Conventional Commits +validator -- it would reject these legitimate downstream commits. + +## Downstream Make Targets + +| Target | Purpose | +|---|---| +| `update-collections` | Update Ansible collections in `openshift/release/ansible/` | +| `generate-requirements` | Regenerate downstream requirements files | +| `check-requirements` | Validate downstream requirements are consistent | +| `check-collections` | Validate Ansible collections are up to date | + diff --git a/docs/references/ecosystem.md b/docs/references/ecosystem.md new file mode 100644 index 00000000..5dd17870 --- /dev/null +++ b/docs/references/ecosystem.md @@ -0,0 +1,18 @@ +# Ecosystem References + +## Operator SDK + +- [operator-framework/operator-sdk](https://github.com/operator-framework/operator-sdk) -- the parent project consuming this plugin +- [operator-framework/operator-lib](https://github.com/operator-framework/operator-lib) -- predicates, instrumented handlers, owner annotations + +## Kubernetes and Controller Runtime + +- [controller-runtime](https://pkg.go.dev/sigs.k8s.io/controller-runtime) v0.21.0 -- controller lifecycle, manager, cache, client +- [kubebuilder](https://pkg.go.dev/sigs.k8s.io/kubebuilder/v4) v4.6.0 -- plugin framework for scaffolding +- [client-go](https://pkg.go.dev/k8s.io/client-go) -- Kubernetes API client, discovery, rest config + +## Ansible + +- [ansible-runner](https://ansible-runner.readthedocs.io/) -- subprocess interface for running playbooks/roles +- Ansible collections for Kubernetes: `kubernetes.core`, `operator_sdk.util` + diff --git a/docs/references/security.md b/docs/references/security.md new file mode 100644 index 00000000..c545f68e --- /dev/null +++ b/docs/references/security.md @@ -0,0 +1,59 @@ +# Security + +This document captures security conventions specific to the ansible-operator-plugins +codebase. For a broader threat analysis, see [THREAT_MODEL.md](../../THREAT_MODEL.md). + +## Proxy Authorization Model + +The proxy uses HTTP Basic Auth not for authentication but as a transport for +owner reference metadata. The username field carries a base64-encoded JSON +`NamespacedOwnerReference`; the password is always `"unused"`. + +1. The `Authorization` header from Ansible must always be stripped before + reaching the Kubernetes API server. Both `removeAuthorizationHeader` and + `RequestLogHandler` strip it. Never remove either call. + +2. Encoding asymmetry: `base64.StdEncoding` for decoding, `base64.URLEncoding` + for encoding. This is intentional. + +3. `getRequestOwnerRef` returns `(nil, nil)` when no Basic Auth is present. + Handle this nil-owner case explicitly. + +## Kubeconfig Generation + +1. Kubeconfig files are created via `os.CreateTemp` and must be deleted after + ansible-runner completes. The reconciler handles cleanup with + `defer os.Remove(kc.Name())`. + +2. The kubeconfig template uses `html/template` (not `text/template`) to + prevent injection through owner reference fields. Do not switch. + +3. The proxy URL must always point to `localhost`. Never change to `0.0.0.0` + or a routable address. + +## Network Binding + +- Kubernetes API proxy: `localhost:8888` (no external exposure) +- Metrics API server: `localhost:5050` (no external exposure) +- Unix sockets for event API: `/tmp` with umask `0077` +- HTTP/2 disabled by default (`--enable-http2` flag, default `false`) +- All HTTP servers set `ReadHeaderTimeout: 5 * time.Second` + +## Owner Reference Injection + +- Only on POST (create), never PUT/PATCH or subresource requests. +- Scope validation via `SupportsOwnerReference` before injection. +- Virtual resources return HTTP 500 rather than silently skipping. +- Watch registration restricted to `watchedNamespaces`. + +## Input Validation + +1. `watches.yaml` validates GVK, paths, finalizer names, and duplicate detection. +2. `markUnsafe` wraps string values as `{"__ansible_unsafe": value}` for Jinja2 protection. +3. Ansible verbosity bounded to 0-7. Max runner artifacts falls back to default on parse failure. + +## File Permissions + +- Runner input directories: `os.ModePerm` (0777) for directories, `0644` for files. +- Project utility constants: `DirMode = 0755`, `FileMode = 0644`, `ExecFileMode = 0755`. + diff --git a/examples/README.md b/examples/README.md new file mode 100644 index 00000000..aad8ce02 --- /dev/null +++ b/examples/README.md @@ -0,0 +1,34 @@ +# Examples + +## Sample Ansible Operator + +`testdata/memcached-molecule-operator/` is a generated, buildable sample +operator produced by `make generate` (via `ansible-operator init` + +scaffolding). It is the canonical reference for: + +- `watches.yaml` structure (GVK-to-role/playbook mapping) +- Role layout under `roles//tasks/main.yml` +- Molecule test scaffolding under `molecule/` +- Generated manifests under `config/` (CRD, RBAC, manager, samples) + +Do not hand-edit files under `testdata/` — see `docs/domain/generated-artifacts.md` +for regeneration rules. + +## Minimal watches.yaml + +```yaml +--- +- version: v1alpha1 + group: cache.example.com + kind: Memcached + playbook: playbooks/memcached.yml + finalizer: + name: cache.example.com/finalizer + role: memfin +``` + +## Further Reading + +- `docs/patterns/README.md` — index of reference implementations by change type +- `docs/AOP_DEVELOPMENT.md` — build, setup, and validation commands +- `AGENTS.md` — agent-facing documentation router diff --git a/hack/generate/samples/ansible/memcached_molecule.go b/hack/generate/samples/ansible/memcached_molecule.go index 6a32f16b..55d116dc 100644 --- a/hack/generate/samples/ansible/memcached_molecule.go +++ b/hack/generate/samples/ansible/memcached_molecule.go @@ -17,7 +17,6 @@ package ansible import ( "fmt" "os" - "os/exec" "path/filepath" "strings" @@ -99,9 +98,10 @@ func ImplementMemcachedMolecule(sample sample.Sample, image string) { pkg.CheckError("replacing the watches file", err) log.Info("removing molecule test for the Secret since it is a core type") - cmd := exec.Command("rm", "-rf", filepath.Join(sample.Dir(), "molecule", "default", "tasks", "secret_test.yml")) - _, err = sample.CommandContext().Run(cmd) - pkg.CheckError("removing secret test file", err) + secretTestPath := filepath.Join(sample.Dir(), "molecule", "default", "tasks", "secret_test.yml") + if err := os.Remove(secretTestPath); err != nil && !os.IsNotExist(err) { + pkg.CheckError("removing secret test file", err) + } log.Info("adding Secret task to the role") err = kbutil.ReplaceInFile(filepath.Join(sample.Dir(), "roles", "secret", "tasks", "main.yml"), @@ -116,7 +116,8 @@ func ImplementMemcachedMolecule(sample sample.Sample, image string) { // prevent high load of controller caused by watching all the secrets in the cluster watchNamespacePatchFileName := "watch_namespace_patch.yaml" log.Info("adding WATCH_NAMESPACE env patch to watch own namespace") - err = os.WriteFile(filepath.Join(sample.Dir(), "config", "testing", watchNamespacePatchFileName), []byte(watchNamespacePatch), 0644) + watchNamespacePatchPath := filepath.Join(sample.Dir(), "config", "testing", watchNamespacePatchFileName) + err = os.WriteFile(watchNamespacePatchPath, []byte(watchNamespacePatch), 0600) pkg.CheckError("adding watch_namespace_patch.yaml", err) log.Info("adding WATCH_NAMESPACE env patch to patch list to be applied") diff --git a/hack/generate/samples/internal/pkg/utils.go b/hack/generate/samples/internal/pkg/utils.go index a1a16848..e5b675e3 100644 --- a/hack/generate/samples/internal/pkg/utils.go +++ b/hack/generate/samples/internal/pkg/utils.go @@ -80,7 +80,7 @@ func removeAllAnnotationLines(annotations map[string]string, filePaths []string) for _, re := range annotationREs { b = re.ReplaceAll(b, []byte{}) } - err = os.WriteFile(file, b, 0644) + err = os.WriteFile(file, b, 0600) if err != nil { return err } diff --git a/internal/ansible/apiserver/apiserver.go b/internal/ansible/apiserver/apiserver.go index 2b035fba..46daee4b 100644 --- a/internal/ansible/apiserver/apiserver.go +++ b/internal/ansible/apiserver/apiserver.go @@ -29,11 +29,13 @@ import ( var log = logf.Log.WithName("apiserver") +// Options configures the user-metrics HTTP server (address and port). type Options struct { Address string Port int } +// Run starts the user-metrics HTTP server on the configured address and port. func Run(options Options) error { mux := http.NewServeMux() mux.HandleFunc("/metrics", metricsHandler) diff --git a/internal/ansible/controller/reconcile.go b/internal/ansible/controller/reconcile.go index 9371a3b2..8b690f41 100644 --- a/internal/ansible/controller/reconcile.go +++ b/internal/ansible/controller/reconcile.go @@ -19,7 +19,6 @@ import ( "encoding/json" "errors" "fmt" - "math/rand" "os" "strconv" "strings" @@ -76,7 +75,7 @@ func (r *AnsibleOperatorReconciler) Reconcile(ctx context.Context, request recon if err != nil { return reconcile.Result{}, err } - ident := strconv.Itoa(rand.Int()) + ident := strconv.FormatInt(time.Now().UnixNano(), 10) logger := logf.Log.WithName("reconciler").WithValues( "job", ident, "name", u.GetName(), diff --git a/internal/ansible/events/log_events.go b/internal/ansible/events/log_events.go index 96a91776..7b59d36d 100644 --- a/internal/ansible/events/log_events.go +++ b/internal/ansible/events/log_events.go @@ -51,6 +51,7 @@ type loggingEventHandler struct { mux *sync.Mutex } +// Handle logs an ansible-runner job event at the configured verbosity level. func (l loggingEventHandler) Handle(ident string, u *unstructured.Unstructured, e eventapi.JobEvent) { if l.LogLevel == Nothing { return diff --git a/internal/ansible/metrics/metrics.go b/internal/ansible/metrics/metrics.go index ba808a57..c0963f80 100644 --- a/internal/ansible/metrics/metrics.go +++ b/internal/ansible/metrics/metrics.go @@ -80,11 +80,13 @@ func recoverMetricPanic() { } } +// RegisterBuildInfo registers the build_info gauge with the given Prometheus registerer. func RegisterBuildInfo(r prometheus.Registerer) { buildInfo.Set(1) r.MustRegister(buildInfo) } +// UserMetric represents a user-defined Prometheus metric submitted via the metrics API. type UserMetric struct { Name string `json:"name" yaml:"name"` Help string `json:"description" yaml:"description"` @@ -94,11 +96,13 @@ type UserMetric struct { Summary *UserMetricSummary `json:"summary,omitempty" yaml:"summary,omitempty"` } +// UserMetricCounter holds the operation to apply to a counter metric. type UserMetricCounter struct { Inc bool `json:"increment,omitempty" yaml:"increment,omitempty"` Add float64 `json:"add,omitempty" yaml:"add,omitempty"` } +// UserMetricGauge holds the operation to apply to a gauge metric. type UserMetricGauge struct { Set float64 `json:"set,omitempty" yaml:"set,omitempty"` Inc bool `json:"increment,omitempty" yaml:"increment,omitempty"` @@ -108,10 +112,12 @@ type UserMetricGauge struct { Sub float64 `json:"subtract,omitempty" yaml:"subtract,omitempty"` } +// UserMetricHistogram holds the observation value for a histogram metric. type UserMetricHistogram struct { Observe float64 `json:"observe,omitempty" yaml:"observe,omitempty"` } +// UserMetricSummary holds the observation value for a summary metric. type UserMetricSummary struct { Observe float64 `json:"observe,omitempty" yaml:"observe,omitempty"` } @@ -219,6 +225,7 @@ func ensureMetric(r prometheus.Registerer, metricSpec UserMetric) { } } +// HandleUserMetric validates, registers (if needed), and applies the operation described by metricSpec. func HandleUserMetric(r prometheus.Registerer, metricSpec UserMetric) error { if err := validateMetricSpec(metricSpec); err != nil { return err @@ -244,17 +251,20 @@ func HandleUserMetric(r prometheus.Registerer, metricSpec UserMetric) error { return nil } +// ReconcileSucceeded increments the successful reconciliation counter for the given GVK. func ReconcileSucceeded(gvk string) { defer recoverMetricPanic() reconcileResults.WithLabelValues(gvk, "succeeded").Inc() } +// ReconcileFailed increments the failed reconciliation counter for the given GVK. func ReconcileFailed(gvk string) { // TODO: consider taking in a failure reason defer recoverMetricPanic() reconcileResults.WithLabelValues(gvk, "failed").Inc() } +// ReconcileTimer returns a Prometheus timer that records reconciliation duration for the given GVK. func ReconcileTimer(gvk string) *prometheus.Timer { defer recoverMetricPanic() return prometheus.NewTimer(prometheus.ObserverFunc(func(duration float64) { diff --git a/internal/ansible/paramconv/paramconv.go b/internal/ansible/paramconv/paramconv.go index fd53dbd2..4a501917 100644 --- a/internal/ansible/paramconv/paramconv.go +++ b/internal/ansible/paramconv/paramconv.go @@ -199,10 +199,12 @@ func convertMapKeys(fn func(string) string, in map[string]interface{}) map[strin return converted } +// MapToSnake recursively converts all map keys from camelCase to snake_case. func MapToSnake(in map[string]interface{}) map[string]interface{} { return convertMapKeys(ToSnake, in) } +// MapToCamel recursively converts all map keys from snake_case to camelCase. func MapToCamel(in map[string]interface{}) map[string]interface{} { return convertMapKeys(ToCamel, in) } diff --git a/internal/ansible/proxy/cache_response.go b/internal/ansible/proxy/cache_response.go index 12807e07..a91b8035 100644 --- a/internal/ansible/proxy/cache_response.go +++ b/internal/ansible/proxy/cache_response.go @@ -56,6 +56,7 @@ type cacheResponseHandler struct { skipPathRegexp []*regexp.Regexp } +// ServeHTTP intercepts GET requests to serve from cache and forwards mutating requests to the API server. func (c *cacheResponseHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { switch req.Method { case http.MethodGet: diff --git a/internal/ansible/proxy/inject_owner.go b/internal/ansible/proxy/inject_owner.go index 901c9b33..0c0114ab 100644 --- a/internal/ansible/proxy/inject_owner.go +++ b/internal/ansible/proxy/inject_owner.go @@ -49,6 +49,7 @@ type injectOwnerReferenceHandler struct { apiResources *apiResources } +// ServeHTTP injects the CR owner reference into create/update requests before forwarding. func (i *injectOwnerReferenceHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { switch req.Method { case http.MethodPost: diff --git a/internal/ansible/proxy/kubeconfig/kubeconfig.go b/internal/ansible/proxy/kubeconfig/kubeconfig.go index 3c5d0506..0bf1478a 100644 --- a/internal/ansible/proxy/kubeconfig/kubeconfig.go +++ b/internal/ansible/proxy/kubeconfig/kubeconfig.go @@ -62,6 +62,7 @@ type values struct { Namespace string } +// NamespacedOwnerReference pairs a Kubernetes OwnerReference with its namespace. type NamespacedOwnerReference struct { metav1.OwnerReference Namespace string diff --git a/internal/ansible/proxy/kubectl.go b/internal/ansible/proxy/kubectl.go index 0c661ebc..9d485005 100644 --- a/internal/ansible/proxy/kubectl.go +++ b/internal/ansible/proxy/kubectl.go @@ -139,6 +139,7 @@ func extractHost(header string) (host string) { return host } +// ServeHTTP delegates accepted requests and rejects forbidden ones with 403. func (f *FilterServer) ServeHTTP(rw http.ResponseWriter, req *http.Request) { host := extractHost(req.Host) if f.accept(req.Method, req.URL.Path, host) { @@ -160,6 +161,7 @@ type server struct { type responder struct{} +// Error writes an HTTP 500 response with the error message. func (r *responder) Error(w http.ResponseWriter, req *http.Request, err error) { log.Error(err, "Error while proxying request") http.Error(w, err.Error(), http.StatusInternalServerError) diff --git a/internal/ansible/proxy/proxy.go b/internal/ansible/proxy/proxy.go index 4b0a2b21..b82a6c6d 100644 --- a/internal/ansible/proxy/proxy.go +++ b/internal/ansible/proxy/proxy.go @@ -49,6 +49,8 @@ import ( // This is the default timeout to wait for the cache to respond // todo(shawn-hurley): Eventually this should be configurable const cacheEstablishmentTimeout = 6 * time.Second + +// AutoSkipCacheREList is a comma-separated list of regex patterns for paths that bypass the proxy cache. const AutoSkipCacheREList = "^/api/.*/pods/.*/exec,^/api/.*/pods/.*/attach" // RequestLogHandler - log the requests that come through the proxy. @@ -357,6 +359,7 @@ func (a *apiResources) resetResources() error { return nil } +// IsVirtualResource returns true if the GVK is a virtual (non-persisted) Kubernetes resource. func (a *apiResources) IsVirtualResource(gvk schema.GroupVersionKind) (bool, error) { a.mu.RLock() apiResource, ok := a.gvkToAPIResource[gvk.String()] diff --git a/internal/ansible/proxy/requestfactory/requestinfo.go b/internal/ansible/proxy/requestfactory/requestinfo.go index 8b40ba58..73f0c48a 100644 --- a/internal/ansible/proxy/requestfactory/requestinfo.go +++ b/internal/ansible/proxy/requestfactory/requestinfo.go @@ -90,6 +90,7 @@ var namespaceSubresources = set.New("status", "finalize") // pkg/master/master_test.go, so we never drift var NamespaceSubResourcesForTest = set.New(namespaceSubresources.SortedList()...) +// RequestInfoFactory resolves Kubernetes API request metadata from HTTP requests. type RequestInfoFactory struct { APIPrefixes set.Set[string] // without leading and trailing slashes GrouplessAPIPrefixes set.Set[string] // without leading and trailing slashes @@ -126,7 +127,8 @@ type RequestInfoFactory struct { // /api/{version} // /api // /healthz - +// +// NewRequestInfo returns the information from the http request. func (r *RequestInfoFactory) NewRequestInfo(req *http.Request) (*RequestInfo, error) { //nolint:gocyclo // TODO: Try to reduce the complexity of this last measured at 33 (failing at > 30) and remove the // nolint:gocyclo // start with a non-resource request until proven otherwise diff --git a/internal/ansible/runner/eventapi/eventapi.go b/internal/ansible/runner/eventapi/eventapi.go index cb75aa0b..cb76686f 100644 --- a/internal/ansible/runner/eventapi/eventapi.go +++ b/internal/ansible/runner/eventapi/eventapi.go @@ -63,6 +63,7 @@ type EventReceiver struct { logger logr.Logger } +// New creates an EventReceiver that listens for ansible-runner events on a Unix socket. func New(ident string, errChan chan<- error) (*EventReceiver, error) { sockPath := fmt.Sprintf("/tmp/ansibleoperator-%s", ident) listener, err := net.Listen("unix", sockPath) diff --git a/internal/ansible/runner/fake/runner.go b/internal/ansible/runner/fake/runner.go index 5cc2152c..c2bd3e1e 100644 --- a/internal/ansible/runner/fake/runner.go +++ b/internal/ansible/runner/fake/runner.go @@ -44,10 +44,12 @@ type runResult struct { stdout string } +// Events returns the channel of job events for this fake run result. func (r *runResult) Events() <-chan eventapi.JobEvent { return r.events } +// Stdout returns the captured standard output of the fake run. func (r *runResult) Stdout() (string, error) { if r.stdout != "" { return r.stdout, nil diff --git a/internal/ansible/runner/internal/inputdir/inputdir.go b/internal/ansible/runner/internal/inputdir/inputdir.go index 48490a46..d4370008 100644 --- a/internal/ansible/runner/internal/inputdir/inputdir.go +++ b/internal/ansible/runner/internal/inputdir/inputdir.go @@ -55,6 +55,9 @@ func (i *InputDir) makeDirs() error { // addFile adds a file to the given relative path within the input directory. func (i *InputDir) addFile(path string, content []byte) error { fullPath := filepath.Join(i.Path, path) + // #nosec G306 -- tracked in THREAT_MODEL.md mitigations (T11/T14); permissions + // of the runner input directory are an intentional, tracked open item and + // should not be tightened as an incidental side effect of a lint cleanup. err := os.WriteFile(fullPath, content, 0644) if err != nil { log.Error(err, "Unable to write file", "Path", fullPath) diff --git a/internal/ansible/runner/runner.go b/internal/ansible/runner/runner.go index 9657d62b..8958db0c 100644 --- a/internal/ansible/runner/runner.go +++ b/internal/ansible/runner/runner.go @@ -85,6 +85,8 @@ func playbookCmdFunc(path string) cmdFuncType { if verbosity > 0 { cmdOptions = append(cmdOptions, ansibleVerbosityString(verbosity)) } + // #nosec G204 -- ansible-runner is a fixed binary name; args are built from + // the operator's own watches.yaml configuration, not external/network input. return exec.Command("ansible-runner", append(cmdArgs, cmdOptions...)...) } } @@ -113,6 +115,8 @@ func roleCmdFunc(path string) cmdFuncType { if ansibleGathering == "explicit" { cmdOptions = append(cmdOptions, "--role-skip-facts") } + // #nosec G204 -- ansible-runner is a fixed binary name; args are built from + // the operator's own watches.yaml configuration, not external/network input. return exec.Command("ansible-runner", append(cmdArgs, cmdOptions...)...) } } @@ -179,6 +183,7 @@ type runner struct { ansibleArgs string } +// Run executes an ansible-runner process for the given CR and returns its result. func (r *runner) Run(ident string, u *unstructured.Unstructured, kubeconfig string) (RunResult, error) { if _, err := exec.LookPath(ansibleRunnerBin); err != nil { return nil, err @@ -422,6 +427,7 @@ func escapeAnsibleKey(key string) string { return key } +// GetFinalizer returns the configured finalizer name and whether one is set. func (r *runner) GetFinalizer() (string, bool) { if r.Finalizer != nil { return r.Finalizer.Name, true diff --git a/internal/testutils/olm.go b/internal/testutils/olm.go index 0a070e7e..f14c1042 100644 --- a/internal/testutils/olm.go +++ b/internal/testutils/olm.go @@ -66,7 +66,7 @@ func (tc TestContext) AddPackagemanifestsTarget(operatorType projutil.OperatorTy // update makefile by adding the packagemanifests target makefileBytes = append([]byte(makefilePackagemanifestsFragment), makefileBytes...) - err = os.WriteFile(filepath.Join(tc.Dir, "Makefile"), makefileBytes, 0644) + err = os.WriteFile(filepath.Join(tc.Dir, "Makefile"), makefileBytes, 0600) if err != nil { return err } diff --git a/internal/testutils/scorecard.go b/internal/testutils/scorecard.go index fcc1a6fe..22666d54 100644 --- a/internal/testutils/scorecard.go +++ b/internal/testutils/scorecard.go @@ -63,7 +63,7 @@ func (tc TestContext) AddScorecardCustomPatchFile() error { // drop in the patch file customScorecardPatchFile := filepath.Join(tc.Dir, "config", "scorecard", "patches", "custom.config.yaml") patchBytes := []byte(customScorecardPatch) - err := os.WriteFile(customScorecardPatchFile, patchBytes, 0777) + err := os.WriteFile(customScorecardPatchFile, patchBytes, 0600) if err != nil { fmt.Printf("can not write %s %s\n", customScorecardPatchFile, err.Error()) return err diff --git a/internal/util/bundleutil/bundleutil.go b/internal/util/bundleutil/bundleutil.go index 7d5012e8..58f9edc9 100644 --- a/internal/util/bundleutil/bundleutil.go +++ b/internal/util/bundleutil/bundleutil.go @@ -221,6 +221,8 @@ func (meta *BundleMetaData) BuildBundleImage(tag string) error { commandArg := strings.Split(meta.BuildCommand, " ") // append the tag and build context to the command + // #nosec G204 -- commandArg comes from the operator author's own bundle metadata + // config, not external/network input. cmd := exec.Command(commandArg[0], append(commandArg[1:], img)...) output, err := cmd.CombinedOutput() if err != nil || viper.GetBool(flags.VerboseOpt) { @@ -265,8 +267,8 @@ func (meta *BundleMetaData) WriteScorecardConfig(inputConfigPath string) error { return err } - err = os.WriteFile(filepath.Join(scorecardDir, "config.yaml"), b, 0644) - if err != nil { + outPath := filepath.Join(scorecardDir, "config.yaml") + if err := os.WriteFile(outPath, b, 0600); err != nil { return fmt.Errorf("error writing scorecard config %v", err) } return nil diff --git a/pkg/testutils/e2e/helpers.go b/pkg/testutils/e2e/helpers.go index 5d2f13b7..f340ee6e 100644 --- a/pkg/testutils/e2e/helpers.go +++ b/pkg/testutils/e2e/helpers.go @@ -43,9 +43,6 @@ func AllowProjectBeMultiGroup(sample sample.Sample) error { } projectBytes = append([]byte(multiGroup), projectBytes...) - err = os.WriteFile(filepath.Join(sample.Dir(), "PROJECT"), projectBytes, 0644) - if err != nil { - return err - } - return nil + projectPath := filepath.Join(sample.Dir(), "PROJECT") + return os.WriteFile(projectPath, projectBytes, 0600) } diff --git a/pkg/testutils/e2e/prometheus/helpers.go b/pkg/testutils/e2e/prometheus/helpers.go index b8f1273d..145fe88e 100644 --- a/pkg/testutils/e2e/prometheus/helpers.go +++ b/pkg/testutils/e2e/prometheus/helpers.go @@ -9,7 +9,7 @@ import ( // InstallPrometheusOperator will install the Prometheus operator onto a Kubernetes cluster func InstallPrometheusOperator(kubectl kubernetes.Kubectl) error { - url, err := getPrometheusOperatorUrl(kubectl) + url, err := getPrometheusOperatorURL(kubectl) if err != nil { return fmt.Errorf("encountered an error when getting the bundle URL: %w", err) } @@ -24,7 +24,7 @@ func InstallPrometheusOperator(kubectl kubernetes.Kubectl) error { // UninstallPrometheusOperator will uninstall a Prometheus operator from a Kubernetes cluster func UninstallPrometheusOperator(kubectl kubernetes.Kubectl) error { - url, err := getPrometheusOperatorUrl(kubectl) + url, err := getPrometheusOperatorURL(kubectl) if err != nil { return fmt.Errorf("encountered an error when getting the bundle URL: %w", err) } @@ -36,9 +36,9 @@ func UninstallPrometheusOperator(kubectl kubernetes.Kubectl) error { return nil } -// getPrometheusOperatorUrl is a helper function to determine the Prometheus +// getPrometheusOperatorURL is a helper function to determine the Prometheus // operator that should be installed on a cluster based on the Kubernetes version -func getPrometheusOperatorUrl(kubectl kubernetes.Kubectl) (string, error) { +func getPrometheusOperatorURL(kubectl kubernetes.Kubectl) (string, error) { prometheusOperatorLegacyVersion := "0.33" prometheusOperatorLegacyURL := "https://raw.githubusercontent.com/coreos/prometheus-operator/release-%s/bundle.yaml" prometheusOperatorVersion := "0.51" diff --git a/pkg/testutils/kubernetes/version.go b/pkg/testutils/kubernetes/version.go index ffe455aa..b1d7ac6b 100644 --- a/pkg/testutils/kubernetes/version.go +++ b/pkg/testutils/kubernetes/version.go @@ -23,8 +23,8 @@ type KubernetesVersion interface { ServerVersion() VersionInfo } -// kubeVersionInfoJson is a struct that allows for easier parsing of the JSON version information from something like `kubectl version` -type kubeVersionInfoJson struct { +// kubeVersionInfoJSON is a struct that allows for easier parsing of the JSON version information from something like `kubectl version` +type kubeVersionInfoJSON struct { Major string `json:"major"` Minor string `json:"minor"` GitVersion string `json:"gitVersion"` @@ -32,14 +32,14 @@ type kubeVersionInfoJson struct { // KubeVersionInfo is an implementation of the VersionInfo interface type KubeVersionInfo struct { - kubeVersionInfoJson + kubeVersionInfoJSON } // NewKubeVersionInfo will return a KubeVersionInfo from a given JSON string func NewKubeVersionInfo(out string) (*KubeVersionInfo, error) { kvi := &KubeVersionInfo{} dec := json.NewDecoder(strings.NewReader(out)) - if err := dec.Decode(&kvi.kubeVersionInfoJson); err != nil { + if err := dec.Decode(&kvi.kubeVersionInfoJSON); err != nil { return nil, err } @@ -48,17 +48,17 @@ func NewKubeVersionInfo(out string) (*KubeVersionInfo, error) { // Major returns the string representation of the Major version func (kvi *KubeVersionInfo) Major() string { - return kvi.kubeVersionInfoJson.Major + return kvi.kubeVersionInfoJSON.Major } // Minor returns the string representatiion of the Minor version func (kvi *KubeVersionInfo) Minor() string { - return kvi.kubeVersionInfoJson.Minor + return kvi.kubeVersionInfoJSON.Minor } // GitVersion returns the string representation of the GitVersion func (kvi *KubeVersionInfo) GitVersion() string { - return kvi.kubeVersionInfoJson.GitVersion + return kvi.kubeVersionInfoJSON.GitVersion } // KubeVersion is an implementation of the KubernetesVersion interface @@ -74,7 +74,7 @@ type KubeVersionOptions func(kv *KubeVersion) func WithClientVersion(clientVersion VersionInfo) KubeVersionOptions { return func(kv *KubeVersion) { kv.clientVersion = KubeVersionInfo{ - kubeVersionInfoJson: kubeVersionInfoJson{ + kubeVersionInfoJSON: kubeVersionInfoJSON{ Major: clientVersion.Major(), Minor: clientVersion.Minor(), GitVersion: clientVersion.GitVersion(), @@ -87,7 +87,7 @@ func WithClientVersion(clientVersion VersionInfo) KubeVersionOptions { func WithServerVersion(serverVersion VersionInfo) KubeVersionOptions { return func(kv *KubeVersion) { kv.serverVersion = KubeVersionInfo{ - kubeVersionInfoJson: kubeVersionInfoJson{ + kubeVersionInfoJSON: kubeVersionInfoJSON{ Major: serverVersion.Major(), Minor: serverVersion.Minor(), GitVersion: serverVersion.GitVersion(), diff --git a/pkg/testutils/sample/generator.go b/pkg/testutils/sample/generator.go index 2c1f0ad7..c7611aae 100644 --- a/pkg/testutils/sample/generator.go +++ b/pkg/testutils/sample/generator.go @@ -10,8 +10,8 @@ type Generator struct { webhook bool preInit GeneratorHook postInit GeneratorHook - preApi GeneratorHook - postApi GeneratorHook + preAPI GeneratorHook + postAPI GeneratorHook preWebhook GeneratorHook postWebhook GeneratorHook } @@ -60,14 +60,14 @@ func WithPostInitHook(hook GeneratorHook) GeneratorOptions { // WithPreApiHook will configure a Generator to run the given GeneratorHook before executing the GenerateApi function of a Sample func WithPreApiHook(hook GeneratorHook) GeneratorOptions { return func(g *Generator) { - g.preApi = hook + g.preAPI = hook } } // WithPostApiHook will configure a Generator to run the given GeneratorHook after executing the GenerateApi function of a Sample func WithPostApiHook(hook GeneratorHook) GeneratorOptions { return func(g *Generator) { - g.postApi = hook + g.postAPI = hook } } @@ -96,8 +96,8 @@ func NewGenerator(opts ...GeneratorOptions) *Generator { webhook: true, preInit: defaultHook, postInit: defaultHook, - preApi: defaultHook, - postApi: defaultHook, + preAPI: defaultHook, + postAPI: defaultHook, preWebhook: defaultHook, postWebhook: defaultHook, } @@ -123,12 +123,12 @@ func (g *Generator) GenerateSamples(samples ...Sample) error { } if g.api { - g.preApi(sample) + g.preAPI(sample) err := sample.GenerateApi() if err != nil { return fmt.Errorf("error in api generation for sample %s: %w", sample.Name(), err) } - g.postApi(sample) + g.postAPI(sample) } if g.webhook { diff --git a/test/common/sa_secret.go b/test/common/sa_secret.go index c33842cd..da425ebf 100644 --- a/test/common/sa_secret.go +++ b/test/common/sa_secret.go @@ -33,7 +33,7 @@ metadata: func GetSASecret(name string, dir string) (string, error) { secretName := name + "-secret" fileName := dir + "/" + secretName + ".yaml" - err := os.WriteFile(fileName, []byte(fmt.Sprintf(saSecretTemplate, secretName, name)), 0777) + err := os.WriteFile(fileName, []byte(fmt.Sprintf(saSecretTemplate, secretName, name)), 0600) if err != nil { return "", err }