Add gitops controller

[machacekondra]

Implements: https://github.com/dcm-project/enhancements/blob/main/enhancements/gitops-controller/gitops-controller.md

[qodo-code-review]

PR Summary by Qodo

Add GitOps controller and GitRepository API

✨ Enhancement 🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Add GitRepository OpenAPI (v1alpha1) plus generated server/client/types code.
• Implement GitRepository persistence (GORM), service validation, and HTTP handlers; wire into
 dcm-server.
• Introduce dcm-gitops controller binary to poll repos, fetch Git, parse YAML, and reconcile
 instances.

Diagram

graph TD
  A["dcm-server"] --> B["GitOps API"] --> C["GitRepo service"] --> D["GitOps store"] --> E[("DB")]
  F["dcm-gitops"] --> G["Controller loop"] --> D
  G --> H{{"Git remote"}}
  G --> I["Catalog service"]

  subgraph Legend
    direction LR
    _svc["Component"] ~~~ _db[("Database")] ~~~ _ext{{"External"}}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Event-driven reconcile queue

• ➕ Avoids DB polling load and reduces reconcile latency for newly created/updated repos
• ➕ Natural place to apply retries/backoff and per-repo rate limiting
• ➖ Requires additional infra (queue) and a producer path from API writes
• ➖ More moving parts than a simple poll loop

2. Use controller-runtime style reconciler framework

• ➕ Well-known patterns for reconciliation, rate limiting, and work queues
• ➕ Easier to extend toward Kubernetes-native GitOps later
• ➖ Adds dependency/complexity; may be overkill if remaining non-K8s
• ➖ Requires adapting current domain/service wiring to the framework model

3. Shell out to system git (instead of go-git)

• ➕ Often more feature-complete and closer to user expectations (auth, protocols, edge cases)
• ➕ Potentially faster for large repos depending on environment
• ➖ Harder to sandbox; requires git in runtime image
• ➖ More error parsing and platform-specific behavior

Recommendation: The PR’s approach (polling controller + go-git + OpenAPI-generated HTTP surface) is a solid MVP for a new subsystem and keeps operational dependencies low. If this will run at higher scale or needs near-real-time sync, consider moving to an event-driven queue later; the current code structure (store/service/controller split) should make that migration straightforward.

Files changed (35) +5329 / -26

Enhancement (21) +4557 / -0

openapi.yamlDefine GitRepository v1alpha1 OpenAPI specification +566/-0

Define GitRepository v1alpha1 OpenAPI specification

• Adds the GitOps Manager API spec describing GitRepository CRUD endpoints, pagination parameters, and shared error schemas. Defines GitRepository spec/status fields used by the API and controller.

api/gitops/v1alpha1/openapi.yaml

spec.gen.goGenerated embedded OpenAPI spec for gitops/v1alpha1 +172/-0

Generated embedded OpenAPI spec for gitops/v1alpha1

• Adds generated Go code that embeds the GitOps OpenAPI spec and exposes GetSpec() for runtime validation.

api/gitops/v1alpha1/spec.gen.go

types.gen.goGenerated GitOps API types (GitRepository, errors, enums) +242/-0

Generated GitOps API types (GitRepository, errors, enums)

• Adds generated Go types and enums for GitRepository and standard error responses used by server/client code.

api/gitops/v1alpha1/types.gen.go

main.goAdd dcm-gitops entrypoint +11/-0

Add dcm-gitops entrypoint

• Creates a new binary entrypoint that runs the GitOps controller process via internal/gitops/app.

cmd/dcm-gitops/main.go

db.goRegister GitRepository model for server DB migrations +2/-0

Register GitRepository model for server DB migrations

• Adds gitops/store/model.GitRepository to the server-side AutoMigrate model list.

internal/app/db.go

openapi.goEnable OpenAPI request validation for GitOps routes +10/-0

Enable OpenAPI request validation for GitOps routes

• Loads the GitOps OpenAPI spec and routes /api/v1alpha1/git-repositories through the request validator middleware.

internal/app/openapi.go

run.goWire GitRepository API into dcm-server routing +14/-0

Wire GitRepository API into dcm-server routing

• Creates the GitOps datastore/service/handler and mounts generated GitOps routes on the chi router. Extends RouteHandlers to include the GitOps StrictServerInterface.

internal/app/run.go

server.gen.goGenerated GitOps OpenAPI server bindings (chi + strict handler) +997/-0

Generated GitOps OpenAPI server bindings (chi + strict handler)

• Adds generated server interface, request/response types, router registration, and strict handler wrappers for the GitRepository API.

internal/gitops/api/server/server.gen.go

run.goImplement dcm-gitops process wiring and controller startup +179/-0

Implement dcm-gitops process wiring and controller startup

• Adds startup logic for logging, DB connection (Postgres/SQLite), GitOps schema migration, and controller lifecycle with signal-based shutdown. Reuses existing catalog/policy/placement wiring to enable reconcile actions via catalog services.

internal/gitops/app/run.go

controller.goAdd polling controller managing per-repo reconciliation +119/-0

Add polling controller managing per-repo reconciliation

• Implements a periodic loop that lists repositories from the store and reconciles due repos. Adds per-repo mutexes to prevent concurrent reconciles of the same repository.

internal/gitops/controller/controller.go

parser.goParse desired CatalogItemInstances from repo YAML files +156/-0

Parse desired CatalogItemInstances from repo YAML files

• Implements non-recursive directory parsing for .yaml/.yml files into DesiredInstance structures with last-file-wins semantics for duplicate names. Returns both parsed instances and per-file parse errors for safe reconciliation decisions.

internal/gitops/controller/parser.go

reconciler.goAdd reconciler to apply Git state to catalog instances +207/-0

Add reconciler to apply Git state to catalog instances

• Implements clone/fetch with timeout, commit change detection, YAML parsing, and create/delete classification. Updates GitRepository sync status in the store and aborts on total parse failure to avoid destructive reconciliation.

internal/gitops/controller/reconciler.go

repository.goAdd go-git based clone/fetch client with safe work dirs +117/-0

Add go-git based clone/fetch client with safe work dirs

• Implements CloneOrFetch and workdir management using go-git, including DNS-1123-style repo ID validation and hard reset to origin/<branch>. Provides an interface for testability.

internal/gitops/controller/repository.go

errors.goMap service errors to OpenAPI error responses +106/-0

Map service errors to OpenAPI error responses

• Introduces consistent error payload construction and maps service-level errors to 400/404/409/500 OpenAPI responses.

internal/gitops/handlers/v1alpha1/errors.go

handler.goImplement GitRepository StrictServerInterface handlers +81/-0

Implement GitRepository StrictServerInterface handlers

• Implements list/create/get/update/delete handlers that delegate to the GitRepository service and emit structured responses. Adds basic request-body presence validation and logs lifecycle operations.

internal/gitops/handlers/v1alpha1/handler.go

service.goAdd GitRepository service with validation and model mapping +264/-0

Add GitRepository service with validation and model mapping

• Implements business logic for CRUD, pagination bridging, and API<->model conversion. Validates display name length, URL format, and reconciliation interval/retry bounds; maps store errors to service errors.

internal/gitops/service/service.go

git_repository.goAdd GitRepository GORM store with CRUD, pagination, and status updates +197/-0

Add GitRepository GORM store with CRUD, pagination, and status updates

• Implements persistence for GitRepository including list pagination via page tokens, unique-constraint mapping, and sync-status updates with timestamps. Adds sentinel errors for not found and uniqueness violations.

internal/gitops/store/git_repository.go

git_repository.goAdd GitRepository database model +27/-0

Add GitRepository database model

• Defines the GitRepository schema including unique display_name, sync status fields, and create/update timestamps.

internal/gitops/store/model/git_repository.go

pagination.goAdd base64 page token encoding/decoding helpers +42/-0

Add base64 page token encoding/decoding helpers

• Implements offset-based pagination tokens with validation and a dedicated invalid-token error for consistent error mapping.

internal/gitops/store/pagination.go

store.goAdd GitOps datastore wrapper (Store interface + constructors) +32/-0

Add GitOps datastore wrapper (Store interface + constructors)

• Introduces a GitOps store interface and a concrete DataStore wiring GORM DB to the GitRepository store implementation.

internal/gitops/store/store.go

client.gen.goGenerated GitOps OpenAPI client for GitRepository API +1016/-0

Generated GitOps OpenAPI client for GitRepository API

• Adds generated HTTP client code supporting GitRepository CRUD operations and request editor hooks for callers.

pkg/gitops/client/client.gen.go

Tests (4) +510 / -0

parser_test.goAdd unit tests for YAML parser behavior and edge cases +249/-0

Add unit tests for YAML parser behavior and edge cases

• Covers valid parsing, ordering, duplicate resolution, missing required fields, wrong kinds, non-YAML files, and missing directories using Ginkgo/Gomega.

internal/gitops/controller/parser_test.go

suite_test.goAdd Ginkgo suite bootstrap for controller package +13/-0

Add Ginkgo suite bootstrap for controller package

• Adds the standard Ginkgo test suite registration for controller tests.

internal/gitops/controller/suite_test.go

git_repository_test.goAdd store tests for CRUD, pagination, uniqueness, and sync status +235/-0

Add store tests for CRUD, pagination, uniqueness, and sync status

• Adds SQLite-backed unit tests covering create/get/list pagination, delete, update, and status updates including error cases.

internal/gitops/store/git_repository_test.go

suite_test.goAdd Ginkgo suite bootstrap for store package +13/-0

Add Ginkgo suite bootstrap for store package

• Adds the standard Ginkgo test suite registration for store tests.

internal/gitops/store/suite_test.go

Other (10) +262 / -26

Containerfile.gitopsAdd dedicated container build for dcm-gitops +31/-0

Add dedicated container build for dcm-gitops

• Introduces a multi-stage build producing a minimal runtime image for the new dcm-gitops binary. Creates a writable Git work directory under /data/gitops and sets GIT_WORK_DIR for runtime.

Containerfile.gitops

MakefileWire GitOps make targets and add dcm-gitops build +7/-3

Wire GitOps make targets and add dcm-gitops build

• Includes make/gitops.mk and adds build-gitops plus test-gitops phony targets. Adjusts the default build to use CGO_ENABLED=1 while keeping a static build for dcm-gitops.

Makefile

spec.gen.cfgAdd oapi-codegen config for embedding GitOps OpenAPI spec +6/-0

Add oapi-codegen config for embedding GitOps OpenAPI spec

• Adds codegen configuration for producing spec.gen.go used by request validation and tooling.

api/gitops/v1alpha1/spec.gen.cfg

types.gen.cfgAdd oapi-codegen config for GitOps API types +6/-0

Add oapi-codegen config for GitOps API types

• Adds codegen configuration for producing typed request/response models from the OpenAPI spec.

api/gitops/v1alpha1/types.gen.cfg

go.modAdd GitOps dependencies (go-git, yaml) and updates +27/-9

Add GitOps dependencies (go-git, yaml) and updates

• Adds go-git for Git operations and yaml.v3 for parsing desired state YAMLs; adjusts related transitive dependencies. Promotes pgx to a direct dependency as part of module tidying/updates.

go.mod

go.sumUpdate module checksums for new GitOps dependencies +83/-14

Update module checksums for new GitOps dependencies

• Adds and updates go.sum entries corresponding to go-git, yaml, and related transitive dependency changes.

go.sum

server.gen.cfgAdd oapi-codegen config for GitOps server stubs +10/-0

Add oapi-codegen config for GitOps server stubs

• Adds generation configuration for the internal GitOps StrictServerInterface and chi router bindings.

internal/gitops/api/server/server.gen.cfg

config.goAdd dcm-gitops configuration via environment +35/-0

Add dcm-gitops configuration via environment

• Defines envconfig-based settings for DB connection, log level, git work directory, and controller polling interval.

internal/gitops/app/config.go

gitops.mkAdd GitOps codegen and test targets +48/-0

Add GitOps codegen and test targets

• Adds Make targets to generate GitOps types/spec/server/client via oapi-codegen and to run GitOps unit/subsystem tests.

make/gitops.mk

client.gen.cfgAdd oapi-codegen config for GitOps API client generation +9/-0

Add oapi-codegen config for GitOps API client generation

• Adds codegen configuration for producing the public GitOps API client package.

pkg/gitops/client/client.gen.cfg

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Matches by display name ✗ Dismissed 🐞 Bug ≡ Correctness

Description

Reconcile keys existing instances by inst.DisplayName, but keys desired instances by YAML
metadata.name and creates with ID = metadata.name, so it can recreate existing instances and/or
delete the wrong ones when display_name differs from the instance ID. Because DisplayName is
non-unique by contract, multiple instances can collide and overwrite each other in the map,
corrupting create/delete decisions.

Code

internal/gitops/controller/reconciler.go[R90-97]

+	existingByName := make(map[string]string, len(existingInstances)) // name -> id
+	for _, inst := range existingInstances {
+		if inst.Uid == nil {
+			slog.WarnContext(ctx, "Skipping instance with nil UID", "id", repo.ID, "display_name", inst.DisplayName)
+			continue
+		}
+		existingByName[inst.DisplayName] = *inst.Uid
+	}

Evidence

The reconciler uses DisplayName as the identity key, but the API type documents it as mutable and
non-unique; the unique identifier is Uid, and the reconciler itself creates using desired.Name
as the ID. This mismatch guarantees incorrect matching when display_name != id and can cause
collisions even when IDs differ.

internal/gitops/controller/reconciler.go[84-104]
internal/gitops/controller/reconciler.go[154-181]
api/catalog/v1alpha1/types.gen.go[97-126]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The reconciler matches desired state to existing state via `DisplayName`, but desired YAML identity and create ID use `metadata.name`.

## Issue Context
`CatalogItemInstance.DisplayName` is explicitly not unique, while `Uid` is the unique identifier.

## Fix Focus Areas
- internal/gitops/controller/reconciler.go[84-112]
- internal/gitops/controller/reconciler.go[154-181]
- api/catalog/v1alpha1/types.gen.go[97-126]

### Implementation direction
- Build `existingByID` keyed by `*inst.Uid` (or by the stored model ID/path), not by `DisplayName`.
- Compare desired `metadata.name` to existing IDs (since `req.ID = &desired.Name`).
- Keep `DisplayName` only for logging/debug, not identity.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Deletes unrelated instances ✗ Dismissed 🐞 Bug ≡ Correctness

Description

Reconciler.listManagedInstances returns all CatalogItemInstances (without repo ownership filtering
and without paging), so the subsequent delete classification can delete instances not managed by the
GitRepository being reconciled. If the repo directory exists but has zero YAML files (and no parse
errors), desired state is empty and every returned instance is treated as deleted-from-Git.

Code

internal/gitops/controller/reconciler.go[R184-195]

+func (r *Reconciler) listManagedInstances(ctx context.Context, repoID string) ([]catalogv1alpha1.CatalogItemInstance, error) {
+	// List all instances and filter by gitops labels.
+	// A future optimization can add label-based filtering to the store.
+	result, err := r.catalogSvc.List(ctx, catalogservice.CatalogItemInstanceListOptions{})
+	if err != nil {
+		return nil, err
+	}
+
+	// TODO: filter by gitops labels once CatalogItemInstance supports labels.
+	// For now, return all instances — the reconciler will match by name.
+	return result.CatalogItemInstances, nil
+}

Evidence

The reconciler explicitly returns all catalog instances as “managed” and then computes toDelete
for anything not in desiredByName, which becomes catastrophic when desired is empty. The catalog
service List API is paginated (has NextPageToken), but the reconciler calls it once and ignores
pagination, so its view of “existing” is incomplete and unsafe to use for deletes.

internal/gitops/controller/reconciler.go[62-112]
internal/gitops/controller/reconciler.go[184-195]
internal/catalog/service/catalog_item_instance.go[27-39]
internal/catalog/service/catalog_item_instance.go[70-97]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`listManagedInstances` currently returns all catalog instances and ignores pagination, but the reconciler treats the returned set as “instances managed by this repo” and may delete those not present in Git.

## Issue Context
There is no current label/ownership field on `CatalogItemInstance` to identify GitOps-managed instances, so deletion must be gated on a reliable ownership signal.

## Fix Focus Areas
- internal/gitops/controller/reconciler.go[77-112]
- internal/gitops/controller/reconciler.go[184-195]

### Implementation direction
- **Do not delete** instances unless ownership can be proven (repoID -> instanceID mapping).
- Add a gitops store table (or equivalent) that records managed instance IDs per repo when created, and list from that mapping.
- When listing from catalog, **page through** results using `NextPageToken` until empty, or query instances by explicit IDs from the mapping.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Repo path escape ✓ Resolved 🐞 Bug ⛨ Security

Description

ParseCatalogItemInstances uses filepath.Join(dir, specPath) and then reads that directory without
validating that specPath stays within the cloned repo, so specPath containing .. (and/or being
absolute) can point outside the repo checkout. The GitRepository API and validation do not constrain
spec.path to a safe relative directory despite documenting it as “within the repo.”

Code

internal/gitops/controller/parser.go[R69-75]

+	fullPath := filepath.Join(dir, specPath)
+
+	entries, err := os.ReadDir(fullPath)
+	if err != nil {
+		result.Errors = append(result.Errors, ParseError{File: fullPath, Err: err})
+		return result
+	}

Evidence

The parser directly joins and reads specPath from the GitRepository model with no containment
check. The API validation only checks URL/interval and does not validate spec.path, and the OpenAPI
spec documents the intention (“within the repo”) without enforcing it.

internal/gitops/controller/parser.go[62-75]
internal/gitops/service/service.go[115-144]
api/gitops/v1alpha1/openapi.yaml[322-340]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`spec.path` is used to form a filesystem path under the repo workdir, but it is not validated or containment-checked, enabling path escape.

## Issue Context
The OpenAPI spec describes `spec.path` as a directory within the repo, but provides no pattern/constraints, and `validateGitRepository` doesn’t validate it.

## Fix Focus Areas
- internal/gitops/controller/parser.go[66-75]
- internal/gitops/service/service.go[115-144]
- api/gitops/v1alpha1/openapi.yaml[334-340]

### Implementation direction
- In `validateGitRepository`, enforce `spec.path` is relative and clean:
 - reject absolute paths
 - reject paths whose cleaned/rel form starts with `..`
- In `ParseCatalogItemInstances`, after joining, compute `absFull := filepath.Abs(fullPath)` and verify it has `absDir` as a prefix using `filepath.Rel(absDir, absFull)` (must not start with `..`).
- Consider resolving symlinks (or explicitly disallow them) if repo contents can influence traversal.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Zero poll interval panics 🐞 Bug ☼ Reliability

Description

Controller.run calls time.NewTicker(c.pollInterval) without guarding against non-positive
durations, which panics at runtime when pollInterval <= 0. POLL_INTERVAL is an environment
variable with no validation and is converted directly to a duration in internal/gitops/app/run.go,
so a misconfiguration can crash the process on startup.

Code

internal/gitops/controller/controller.go[R56-57]

+	ticker := time.NewTicker(c.pollInterval)
+	defer ticker.Stop()

Evidence

The controller always creates a ticker from pollInterval, while config allows any int value for
POLL_INTERVAL and run.go converts it directly to a time.Duration. This creates a direct crash path
on misconfiguration.

internal/gitops/controller/controller.go[50-58]
internal/gitops/app/config.go[9-16]
internal/gitops/app/run.go[104-107]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`time.NewTicker` panics if its duration is <= 0, but `POLL_INTERVAL` is not validated before constructing the controller.

## Issue Context
This is a startup-time reliability failure caused by configuration.

## Fix Focus Areas
- internal/gitops/controller/controller.go[50-58]
- internal/gitops/app/config.go[9-16]
- internal/gitops/app/run.go[104-107]

### Implementation direction
- Validate `cfg.PollInterval >= 1` in `LoadConfig()` or `Run()` and return a clear error.
- Optionally add a guard in `NewController`/`Controller.Start` to reject non-positive intervals defensively.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[gabriel-farache]

why all and just not 1? What if a previously well formatted file is now corrupted?

[gciavarrini]

Agree and the same risk exists when only some files fail: desired map omits them, so they look “removed from Git” and get deleted.
the enhancement conflict table says "invalid YAML -> skip file / report error" not delete.
Why treat a parse failure as absence?

[machacekondra]

It would bring complexity. We would need to start storing which file produced which instance. Is it ok for first iteration to have at least this error, which basically only detect, permissions/disk issues for example.

[gabriel-farache]

ok for v1 but this must be tracked

[gciavarrini]

Agree and the same risk exists when only some files fail: desired map omits them, so they look “removed from Git” and get deleted.
the enhancement conflict table says "invalid YAML -> skip file / report error" not delete.
Why treat a parse failure as absence?

[gciavarrini]

Why take repoID / commit and discard them as _?
Enhancement says set gitops.dcm.io/repository and gitops.dcm.io/commit on create. Is that deferred to a follow up PR? or should the enhancement be updated?

[machacekondra]

Right, added it so labels are set in CatalogIntance.

[gciavarrini]

On create/delete failure we append to reconcileErrors and later set ERROR. repo.MaxRetries and repo.BackoffSeconds are never used. why ignore them here?

[machacekondra]

Right, AI missed it, I'll remove for now for, easier review.

[gciavarrini]

spec.reconciliation.retry_policy is still in OpenAPI and still not stored or applied.

[gciavarrini]

Enhancement still lists GET …/status and POST …:sync.
The drop seems intentional so we need to update the enhancement, no?

[machacekondra]

Yes and no. Those features can be implemented as follow-ups, if there is a reason to have them, or update enhacenemt. Let's see, but I think it would be better to have it as simple as possible, as DCM still lacks to many things, so it doesn't make sense to make gitops complicated for time being.

[jordigilh]

Not previously flagged: everything downstream of this context — including the catalogSvc.Create/Delete calls the reconciler makes — runs with no caller identity attached. Combined with the unscoped listing above, there's no notion anywhere in this path of which repository (or which user) is allowed to touch which instance.

[machacekondra]

Not sure I understand, there is no authz implemented rn, what do you mean? Also this reconciler never creates/removes catalog ussing catalogSvc.

[jordigilh]

It does — r.catalogSvc is catalogservice.CatalogItemInstanceService (injected via catalogSvc.CatalogItemInstance() in run.go), and the reconciler calls Delete at line 129 and Create at line 173.

Re: authz — not claiming that's new, just that there's no identity plumbing for it to ever attach to here. Combined with the unscoped listManagedInstances above, this path has no way to even express "which repo owns which instance," so it's not just pending auth/z, there's nothing yet to enforce against.