diff --git a/docs/plans-executions/2026-08-07-1747-proxy-operator.md b/docs/plans-executions/2026-08-07-1747-proxy-operator.md index 60595c3..f5dd2bb 100644 --- a/docs/plans-executions/2026-08-07-1747-proxy-operator.md +++ b/docs/plans-executions/2026-08-07-1747-proxy-operator.md @@ -6,7 +6,7 @@ Pairs with [docs/plans/2026-08-07-1747-proxy-operator.md](../plans/2026-08-07-17 - [x] Step 0 — Branch and scaffold - [x] Step 1 — API types (`api/v1alpha1/proxy_types.go`) -- [ ] Step 2 — Provider contract (`internal/provider/`) +- [x] Step 2 — Provider contract (`internal/provider/`) - [ ] Step 3 — Mock provider (`internal/provider/mock/`) - [ ] Step 4 — Reconciler (`internal/controller/`) - [ ] Step 5 — Health engine (`internal/health/`) @@ -148,3 +148,54 @@ which only `make test` does via `setup-envtest`. Plain `go test ./...` fails the `internal/controller` package with a `/usr/local/kubebuilder/bin/etcd: no such file` error that has nothing to do with the code. Use `make test`, not `go test ./...`, whenever the controller package is in scope. + +## Step 2 — Provider contract (`internal/provider/`) + +Wrote the `Provider` interface (`Create`/`Get`/`Delete`/`ListByTag`), `Instance`, +`Placement`, `CreateRequest`, and the GC-contract label constants +(`provider.go`); the error taxonomy with multi-error `Unwrap() []error` so +`errors.Is` and `errors.As` both work off the same wrapped value +(`errors.go`); deterministic instance naming via SHA-256 → base32 → 16 +chars (`name.go`); and `--providers-config` YAML parsing with fail-fast +validation (`config.go`). + +One deliberate deviation from the plan's file layout: the plan listed +`internal/provider/metrics.go` as part of this step, but the +`provider.WithMetrics` decorator it describes is Step 9's concern (it needs +the Prometheus vectors that don't exist until the metrics package is +built) and nothing in this step depends on it existing yet. Deferred to +Step 9 rather than writing a decorator with nowhere to register its +metrics. + +The registry package (`internal/provider/registry/registry.go`) came out +slightly different from the plan's sketch, and better for it: instead of a +package-level `var builtin = map[string]Constructor{"mock": mock.New, "gcp": +gcp.New}` living inside the registry package, `Build` takes the +`map[string]Constructor` as a parameter. This means `registry` has zero +import on `internal/provider/mock` or `internal/provider/gcp` — neither of +which exists yet at this point in the plan (mock is Step 3, gcp is Step 8) +— so the package compiles today instead of only once both are done, and the +explicit wiring lives at the composition root (`cmd/main.go`, Step 10) +rather than being smeared into the registry package itself. Still fully +avoids the import-cycle trap the plan called out. + +Ran the full suite: + +```bash +go mod tidy # sigs.k8s.io/yaml (already an indirect dep of the k8s.io toolchain) promoted to direct +go build ./... && go vet ./... +go test -race -v ./internal/provider/... +make test +``` + +`internal/provider` landed at 96.2% coverage, `internal/provider/registry` at +100%. `make test` also ran `go fmt ./...`, which reformatted `errors.go`'s +struct-field comment alignment before its first commit — no logic change, +just gofmt on a brand-new file. + +Worth noting: `sigs.k8s.io/yaml` (not `gopkg.in/yaml.v3`) was picked for +`--providers-config` parsing specifically because it has `UnmarshalStrict` +built in (rejects unknown fields, which is what "fail fast on unknown type" +in the plan actually needs) and was already pulled in transitively by the +k8s.io toolchain, so no new dependency was added — `go mod tidy` just +promoted it from indirect to direct. diff --git a/go.mod b/go.mod index abcface..e0027aa 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( k8s.io/apimachinery v0.36.0 k8s.io/client-go v0.36.0 sigs.k8s.io/controller-runtime v0.24.1 + sigs.k8s.io/yaml v1.6.0 ) require ( @@ -96,5 +97,4 @@ require ( sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 // indirect sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v6 v6.3.2 // indirect - sigs.k8s.io/yaml v1.6.0 // indirect ) diff --git a/internal/provider/config.go b/internal/provider/config.go new file mode 100644 index 0000000..9793783 --- /dev/null +++ b/internal/provider/config.go @@ -0,0 +1,133 @@ +package provider + +import ( + "fmt" + "os" + + "sigs.k8s.io/yaml" +) + +// Fail-with classes accepted by MockConfig.FailWith, shared with the mock +// provider's fault injection so the two sides never drift on the string +// values. +const ( + FailWithNotFound = "notfound" + FailWithQuota = "quota" + FailWithTransient = "transient" + FailWithPermanent = "permanent" +) + +var validFailClasses = map[string]bool{ + FailWithNotFound: true, + FailWithQuota: true, + FailWithTransient: true, + FailWithPermanent: true, +} + +// Config is the top-level shape of the --providers-config file. +type Config struct { + Providers []ProviderConfig `json:"providers"` +} + +// ProviderConfig is one named, typed provider instance — e.g. "gcp-eu" and +// "gcp-us" can be two ProviderConfigs of Type "gcp" with different GCP +// blocks. Exactly one of the type-specific blocks below should be set, +// matching Type. +type ProviderConfig struct { + Name string `json:"name"` + Type string `json:"type"` + Mock *MockConfig `json:"mock,omitempty"` + GCP *GCPConfig `json:"gcp,omitempty"` +} + +// MockConfig configures the in-memory mock provider. +type MockConfig struct { + // ProvisionDelaySeconds is how long a created instance reports + // Provisioning before Running. Default 5. + ProvisionDelaySeconds int32 `json:"provisionDelaySeconds,omitempty"` + // DeleteDelaySeconds is how long a deleted instance reports Terminated + // before Get starts returning ErrNotFound. Default 1. + DeleteDelaySeconds int32 `json:"deleteDelaySeconds,omitempty"` + // FailNextCreates makes the next N Create calls fail with FailWith, + // for exercising the reconciler's error handling in demos. + FailNextCreates int `json:"failNextCreates,omitempty"` + // FailWith selects the error class injected failures return: one of + // FailWithNotFound/FailWithQuota/FailWithTransient/FailWithPermanent. + // Default FailWithTransient. + FailWith string `json:"failWith,omitempty"` +} + +// GCPConfig configures a named GCP provider instance. +type GCPConfig struct { + // Project is the GCP project ID. Required. + Project string `json:"project"` + // Network is the VPC network name. Default "default". + Network string `json:"network,omitempty"` + // NetworkTag is the firewall/network tag applied to created instances. + // Default "proxy-operator". + NetworkTag string `json:"networkTag,omitempty"` + // DiskSizeGB is the boot disk size in GB. Default 10. + DiskSizeGB int64 `json:"diskSizeGb,omitempty"` +} + +// LoadConfigFile reads and parses a providers-config file from disk. +func LoadConfigFile(path string) (*Config, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("reading providers config %s: %w", path, err) + } + return LoadConfig(data) +} + +// LoadConfig parses and validates providers-config YAML. It fails fast: +// unknown provider types, duplicate names, and missing type-specific +// required fields are all load-time errors here, not runtime surprises +// discovered only when a provider is actually used. +func LoadConfig(data []byte) (*Config, error) { + var cfg Config + if err := yaml.UnmarshalStrict(data, &cfg); err != nil { + return nil, fmt.Errorf("parsing providers config: %w", err) + } + if err := cfg.validate(); err != nil { + return nil, fmt.Errorf("validating providers config: %w", err) + } + return &cfg, nil +} + +func (c *Config) validate() error { + if len(c.Providers) == 0 { + return fmt.Errorf("providers: at least one provider must be configured") + } + seen := make(map[string]bool, len(c.Providers)) + for i, p := range c.Providers { + if p.Name == "" { + return fmt.Errorf("providers[%d]: name is required", i) + } + if seen[p.Name] { + return fmt.Errorf("providers[%d]: duplicate provider name %q", i, p.Name) + } + seen[p.Name] = true + + switch p.Type { + case "mock": + if p.GCP != nil { + return fmt.Errorf("providers[%d] %q: type is mock but a gcp block is set", i, p.Name) + } + if p.Mock != nil && p.Mock.FailWith != "" && !validFailClasses[p.Mock.FailWith] { + return fmt.Errorf("providers[%d] %q: unknown mock.failWith %q", i, p.Name, p.Mock.FailWith) + } + case "gcp": + if p.Mock != nil { + return fmt.Errorf("providers[%d] %q: type is gcp but a mock block is set", i, p.Name) + } + if p.GCP == nil || p.GCP.Project == "" { + return fmt.Errorf("providers[%d] %q: gcp.project is required", i, p.Name) + } + case "": + return fmt.Errorf("providers[%d] %q: type is required", i, p.Name) + default: + return fmt.Errorf("providers[%d] %q: unknown provider type %q", i, p.Name, p.Type) + } + } + return nil +} diff --git a/internal/provider/config_test.go b/internal/provider/config_test.go new file mode 100644 index 0000000..79571cd --- /dev/null +++ b/internal/provider/config_test.go @@ -0,0 +1,173 @@ +package provider + +import ( + "strings" + "testing" +) + +func TestLoadConfig_valid(t *testing.T) { + t.Parallel() + data := []byte(` +providers: + - name: mock + type: mock + mock: + provisionDelaySeconds: 2 + - name: gcp-eu + type: gcp + gcp: + project: my-project + network: custom-net +`) + cfg, err := LoadConfig(data) + if err != nil { + t.Fatalf("LoadConfig() error = %v, want nil", err) + } + if len(cfg.Providers) != 2 { + t.Fatalf("len(cfg.Providers) = %d, want 2", len(cfg.Providers)) + } + if cfg.Providers[0].Mock == nil || cfg.Providers[0].Mock.ProvisionDelaySeconds != 2 { + t.Errorf("providers[0].mock = %+v, want ProvisionDelaySeconds=2", cfg.Providers[0].Mock) + } + if cfg.Providers[1].GCP == nil || cfg.Providers[1].GCP.Project != "my-project" { + t.Errorf("providers[1].gcp = %+v, want Project=my-project", cfg.Providers[1].GCP) + } +} + +func TestLoadConfig_invalid(t *testing.T) { + t.Parallel() + tests := []struct { + name string + yaml string + wantErrSub string + }{ + { + name: "empty providers list", + yaml: `providers: []`, + wantErrSub: "at least one provider", + }, + { + name: "missing name", + yaml: ` +providers: + - type: mock`, + wantErrSub: "name is required", + }, + { + name: "duplicate name", + yaml: ` +providers: + - name: mock + type: mock + - name: mock + type: mock`, + wantErrSub: "duplicate provider name", + }, + { + name: "unknown type", + yaml: ` +providers: + - name: p1 + type: azure`, + wantErrSub: `unknown provider type "azure"`, + }, + { + name: "missing type", + yaml: ` +providers: + - name: p1`, + wantErrSub: "type is required", + }, + { + name: "gcp missing project", + yaml: ` +providers: + - name: gcp-eu + type: gcp`, + wantErrSub: "gcp.project is required", + }, + { + name: "gcp with empty project", + yaml: ` +providers: + - name: gcp-eu + type: gcp + gcp: + project: ""`, + wantErrSub: "gcp.project is required", + }, + { + name: "mock type with gcp block", + yaml: ` +providers: + - name: p1 + type: mock + gcp: + project: my-project`, + wantErrSub: "type is mock but a gcp block is set", + }, + { + name: "gcp type with mock block", + yaml: ` +providers: + - name: p1 + type: gcp + gcp: + project: my-project + mock: + failNextCreates: 1`, + wantErrSub: "type is gcp but a mock block is set", + }, + { + name: "unknown mock.failWith", + yaml: ` +providers: + - name: p1 + type: mock + mock: + failWith: oops`, + wantErrSub: `unknown mock.failWith "oops"`, + }, + { + name: "strict mode rejects unknown top-level key", + yaml: ` +providers: + - name: p1 + type: mock +extraneous: true`, + wantErrSub: "parsing providers config", + }, + { + name: "strict mode rejects unknown provider key", + yaml: ` +providers: + - name: p1 + type: mock + bogus: true`, + wantErrSub: "parsing providers config", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + _, err := LoadConfig([]byte(tc.yaml)) + if err == nil { + t.Fatalf("LoadConfig() error = nil, want error containing %q", tc.wantErrSub) + } + if !strings.Contains(err.Error(), tc.wantErrSub) { + t.Errorf("LoadConfig() error = %q, want substring %q", err.Error(), tc.wantErrSub) + } + }) + } +} + +func TestLoadConfigFile_missingFile(t *testing.T) { + t.Parallel() + _, err := LoadConfigFile("/nonexistent/providers.yaml") + if err == nil { + t.Fatal("LoadConfigFile() error = nil, want error") + } + if !strings.Contains(err.Error(), "reading providers config") { + t.Errorf("LoadConfigFile() error = %q, want substring %q", err.Error(), "reading providers config") + } +} diff --git a/internal/provider/errors.go b/internal/provider/errors.go new file mode 100644 index 0000000..cac51bf --- /dev/null +++ b/internal/provider/errors.go @@ -0,0 +1,91 @@ +package provider + +import ( + "errors" + "fmt" +) + +// The provider error taxonomy. Every error a Provider method returns should +// be classifiable as exactly one of these four, via Wrap or Class. The +// reconciler branches on this classification to decide how to react — +// never on a provider-specific error type. +var ( + // ErrNotFound means the instance doesn't exist. Get returning this is + // normal, not exceptional. + ErrNotFound = errors.New("provider: instance not found") + // ErrQuotaExceeded means the request failed because of a cloud quota or + // rate limit. The reconciler backs off slowly (minutes, not seconds) + // rather than hammering an exhausted quota. + ErrQuotaExceeded = errors.New("provider: quota exceeded") + // ErrTransient means the request failed for a reason likely to clear on + // retry (network blip, 5xx, timeout). The reconciler retries with the + // workqueue's normal exponential backoff. + ErrTransient = errors.New("provider: transient error") + // ErrPermanent means the request failed for a reason that will not + // clear on retry (bad config, permission denied, invalid argument). The + // reconciler stops retrying and surfaces the failure in status. + ErrPermanent = errors.New("provider: permanent error") +) + +// Error wraps a provider-specific error with enough context for logs, while +// remaining classifiable via errors.Is against one of the four sentinels +// above and unwrappable via errors.As to the underlying SDK error. +type Error struct { + Class error // one of ErrNotFound/ErrQuotaExceeded/ErrTransient/ErrPermanent + Op string // "create", "get", "delete", "list" + Provider string // the configured provider name, e.g. "gcp-eu" + ID string // providerID, if known + Err error // underlying error; may be nil +} + +func (e *Error) Error() string { + msg := fmt.Sprintf("provider %s: %s", e.Provider, e.Op) + if e.ID != "" { + msg += fmt.Sprintf(" %s", e.ID) + } + msg += ": " + e.Class.Error() + if e.Err != nil { + msg += fmt.Sprintf(" (%v)", e.Err) + } + return msg +} + +// Unwrap exposes both the taxonomy sentinel and the underlying error, via +// Go's multi-error unwrap (errors.Unwrap() []error). This is what lets +// errors.Is(err, ErrQuotaExceeded) and errors.As(err, &googleErr) both +// succeed against the same *Error value: errors.Is/As walk every branch. +func (e *Error) Unwrap() []error { + if e.Err != nil { + return []error{e.Class, e.Err} + } + return []error{e.Class} +} + +// Wrap builds an *Error classified under class. err is the underlying +// SDK/network error and may be nil when there's nothing further to wrap +// (e.g. classifying a bare HTTP status code). +func Wrap(class error, op, providerName, id string, err error) error { + return &Error{Class: class, Op: op, Provider: providerName, ID: id, Err: err} +} + +// Class returns the taxonomy sentinel matching err — ErrNotFound, +// ErrQuotaExceeded, ErrPermanent, or ErrTransient, checked via errors.Is so +// it works against any error built with Wrap regardless of how deeply it's +// wrapped elsewhere. An unclassified error (nil, or not built with Wrap) +// defaults to ErrTransient: retrying is always safer than latching Failed +// on an error nobody has taught this package to recognize. +func Class(err error) error { + if err == nil { + return nil + } + switch { + case errors.Is(err, ErrNotFound): + return ErrNotFound + case errors.Is(err, ErrQuotaExceeded): + return ErrQuotaExceeded + case errors.Is(err, ErrPermanent): + return ErrPermanent + default: + return ErrTransient + } +} diff --git a/internal/provider/errors_test.go b/internal/provider/errors_test.go new file mode 100644 index 0000000..6e87502 --- /dev/null +++ b/internal/provider/errors_test.go @@ -0,0 +1,70 @@ +package provider + +import ( + "errors" + "fmt" + "testing" +) + +// sdkError simulates an underlying provider SDK error type, so tests can +// assert errors.As reaches through the wrapper to it. +type sdkError struct{ code int } + +func (e *sdkError) Error() string { return fmt.Sprintf("sdk error %d", e.code) } + +func TestError_IsAndAs(t *testing.T) { + t.Parallel() + underlying := &sdkError{code: 429} + err := Wrap(ErrQuotaExceeded, "create", "gcp-eu", "zones/z/instances/x", underlying) + + if !errors.Is(err, ErrQuotaExceeded) { + t.Error("errors.Is(err, ErrQuotaExceeded) = false, want true") + } + if errors.Is(err, ErrNotFound) { + t.Error("errors.Is(err, ErrNotFound) = true, want false") + } + + var sdk *sdkError + if !errors.As(err, &sdk) { + t.Fatal("errors.As(err, &sdk) = false, want true") + } + if sdk.code != 429 { + t.Errorf("recovered sdkError.code = %d, want 429", sdk.code) + } +} + +func TestError_WrapWithNilUnderlying(t *testing.T) { + t.Parallel() + err := Wrap(ErrPermanent, "get", "mock", "id", nil) + if !errors.Is(err, ErrPermanent) { + t.Error("errors.Is(err, ErrPermanent) = false, want true") + } + if err.Error() == "" { + t.Error("Error() returned an empty string") + } +} + +func TestClass(t *testing.T) { + t.Parallel() + tests := []struct { + name string + err error + want error + }{ + {"nil returns nil", nil, nil}, + {"wrapped not found", Wrap(ErrNotFound, "get", "mock", "id", nil), ErrNotFound}, + {"wrapped quota", Wrap(ErrQuotaExceeded, "create", "gcp", "id", nil), ErrQuotaExceeded}, + {"wrapped permanent", Wrap(ErrPermanent, "create", "gcp", "id", nil), ErrPermanent}, + {"wrapped transient", Wrap(ErrTransient, "create", "gcp", "id", nil), ErrTransient}, + {"unclassified defaults to transient", errors.New("boom"), ErrTransient}, + {"further-wrapped preserves classification", fmt.Errorf("outer: %w", Wrap(ErrQuotaExceeded, "create", "gcp", "id", nil)), ErrQuotaExceeded}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if got := Class(tc.err); got != tc.want { + t.Errorf("Class(%v) = %v, want %v", tc.err, got, tc.want) + } + }) + } +} diff --git a/internal/provider/name.go b/internal/provider/name.go new file mode 100644 index 0000000..534eba4 --- /dev/null +++ b/internal/provider/name.go @@ -0,0 +1,33 @@ +package provider + +import ( + "crypto/sha256" + "encoding/base32" + "strings" + + "k8s.io/apimachinery/pkg/types" +) + +const namePrefix = "proxy-" + +// NameFromUID derives a deterministic instance name from a Proxy CR's UID. +// Provider.Create implementations key idempotency on this name: a repeat +// call after a crash must find the existing instance by this name rather +// than create a duplicate, and the reconciler relies on that to recover +// providerID after a crash even if status was wiped. +// +// SHA-256 of the UID, truncated to the first 10 bytes, RFC 4648 base32 +// (no padding), lowercased: exactly 16 characters, for 22 total with the +// "proxy-" prefix. That gives 80 bits of collision resistance — a birthday +// collision only becomes likely around 2^40 objects, against a fleet of +// tens — while satisfying GCP's instance name rules +// (^[a-z]([-a-z0-9]{0,61}[a-z0-9])?$, <=63 chars) with headroom, since GCP +// auto-names some resources (e.g. the boot disk) after the instance name. +// base32's alphabet (A-Z2-7) is entirely legal once lowercased; base64 is +// not (has '+', '/', and uppercase), and hex would need 20 characters for +// the same 80 bits. +func NameFromUID(uid types.UID) string { + sum := sha256.Sum256([]byte(uid)) + enc := base32.StdEncoding.WithPadding(base32.NoPadding) + return namePrefix + strings.ToLower(enc.EncodeToString(sum[:10])) +} diff --git a/internal/provider/name_test.go b/internal/provider/name_test.go new file mode 100644 index 0000000..b253c00 --- /dev/null +++ b/internal/provider/name_test.go @@ -0,0 +1,61 @@ +package provider + +import ( + "fmt" + "regexp" + "testing" + + "k8s.io/apimachinery/pkg/types" +) + +var nameRegexp = regexp.MustCompile(`^proxy-[a-z2-7]{16}$`) + +func TestNameFromUID_idempotent(t *testing.T) { + t.Parallel() + uid := types.UID("f47ac10b-58cc-4372-a567-0e02b2c3d479") + got1 := NameFromUID(uid) + got2 := NameFromUID(uid) + if got1 != got2 { + t.Errorf("NameFromUID(%q) is not idempotent: %q != %q", uid, got1, got2) + } +} + +func TestNameFromUID_charsetAndLength(t *testing.T) { + t.Parallel() + tests := []types.UID{ + "f47ac10b-58cc-4372-a567-0e02b2c3d479", + "", + "a", + "00000000-0000-0000-0000-000000000000", + "ffffffff-ffff-ffff-ffff-ffffffffffff", + } + for _, uid := range tests { + t.Run(string(uid), func(t *testing.T) { + t.Parallel() + name := NameFromUID(uid) + if !nameRegexp.MatchString(name) { + t.Errorf("NameFromUID(%q) = %q, does not match %s", uid, name, nameRegexp) + } + if len(name) > 63 { + t.Errorf("NameFromUID(%q) = %q, length %d exceeds GCP's 63-char limit", uid, name, len(name)) + } + if len(name) != len(namePrefix)+16 { + t.Errorf("NameFromUID(%q) = %q, length %d, want %d", uid, name, len(name), len(namePrefix)+16) + } + }) + } +} + +func TestNameFromUID_distinctAcrossFleet(t *testing.T) { + t.Parallel() + const n = 10000 + seen := make(map[string]types.UID, n) + for i := range n { + uid := types.UID(fmt.Sprintf("00000000-0000-0000-0000-%012d", i)) + name := NameFromUID(uid) + if prior, ok := seen[name]; ok { + t.Fatalf("collision: NameFromUID(%q) == NameFromUID(%q) == %q", uid, prior, name) + } + seen[name] = uid + } +} diff --git a/internal/provider/provider.go b/internal/provider/provider.go new file mode 100644 index 0000000..edcf477 --- /dev/null +++ b/internal/provider/provider.go @@ -0,0 +1,109 @@ +// Package provider defines the contract every cloud provider backend +// implements: Create/Get/Delete/ListByTag for a single VM, keyed by an +// opaque providerID. Concrete implementations live in subpackages (mock, +// gcp); this package has no dependency on any of them, so a type→ +// constructor registry can be assembled at the composition root (cmd/main.go) +// without an import cycle. +package provider + +import ( + "context" + "time" +) + +// InstanceState is a provider's lifecycle state for a single VM. +type InstanceState string + +const ( + StateProvisioning InstanceState = "Provisioning" + StateRunning InstanceState = "Running" + StateStopped InstanceState = "Stopped" + StateTerminated InstanceState = "Terminated" +) + +// GC contract: every cloud resource a provider creates must carry these two +// labels/tags. Orphan GC (internal/gc) relies on both — Managed to find +// resources it owns at all, UID to decide whether a resource is still +// claimed by a live Proxy CR. +const ( + LabelManaged = "proxy-operator-managed" + LabelManagedYes = "true" + LabelUID = "proxy-operator-uid" +) + +// Instance is a provider's view of a single VM. +type Instance struct { + // ID is the opaque providerID, stable for the life of the VM. + ID string + // IP is the VM's current address, empty until it's assigned one. + IP string + // State is the VM's current lifecycle state. + State InstanceState + // UID is the LabelUID value read back off the resource — the owning + // Proxy CR's UID, or "" if the resource predates this label (shouldn't + // happen for anything this operator created, but Get/ListByTag callers + // must tolerate it rather than panic). + UID string + // CreatedAt is when the provider created the resource. Orphan GC uses + // this to skip young instances that may still be mid-create, avoiding a + // race with an in-flight Create whose status write hasn't landed yet. + CreatedAt time.Time +} + +// Placement is the provider-opaque placement/size configuration a Create +// call needs. It deliberately does not import api/v1alpha1 — this package +// stays independent of the CRD types; the reconciler maps +// v1alpha1.PlacementSpec to this struct when calling Create. Providers may +// ignore fields that don't apply to them. +type Placement struct { + Region string + Zone string + MachineType string + Image string +} + +// CreateRequest carries everything a provider needs to create a VM. +type CreateRequest struct { + // Name is the deterministic instance name, already derived from the + // owning CR's UID via NameFromUID. Create must be idempotent keyed on + // this name: a repeat call after a crash must find the existing + // instance rather than create a duplicate. + Name string + // UID is the owning Proxy CR's UID. Create must tag/label the created + // resource with LabelUID=UID and LabelManaged=LabelManagedYes. + UID string + + Namespace string + ProxyName string + + Placement Placement + // CloudInit is the already-resolved user-data (a Secret reference, if + // used, has already been read by the caller). + CloudInit string + Port int32 +} + +// Provider is the contract every cloud backend implements. Kept +// deliberately minimal: this is the same interface five future providers +// must satisfy. +type Provider interface { + // Create starts VM creation and returns as soon as the request is + // submitted — it does not block until the VM is running. Must be + // idempotent by req.Name, so a repeat call after a crash finds the + // existing VM instead of duplicating it. + Create(ctx context.Context, req CreateRequest) (providerID string, err error) + + // Get returns the current state of a previously created instance. + // Returning (nil, ErrNotFound) is a normal, expected outcome — it + // drives the reconciler's replacement and adoption logic, not an + // exceptional condition. + Get(ctx context.Context, providerID string) (*Instance, error) + + // Delete is idempotent: deleting an instance that no longer exists is + // not an error. + Delete(ctx context.Context, providerID string) error + + // ListByTag returns every instance this operator has ever tagged with + // LabelManaged=LabelManagedYes, for orphan GC. + ListByTag(ctx context.Context) ([]Instance, error) +} diff --git a/internal/provider/registry/registry.go b/internal/provider/registry/registry.go new file mode 100644 index 0000000..d3041cf --- /dev/null +++ b/internal/provider/registry/registry.go @@ -0,0 +1,44 @@ +// Package registry assembles a set of named providers from a +// provider.Config by dispatching each entry's Type through a +// caller-supplied map of constructors. +// +// This package deliberately never imports internal/provider/mock or +// internal/provider/gcp. If it did, and internal/provider ever needed to +// import this package (e.g. to expose a default registry), that would be an +// import cycle: internal/provider/mock already imports internal/provider +// for the Provider interface. Keeping the type→constructor map external — +// supplied by the composition root in cmd/main.go — avoids the cycle +// entirely and keeps this package trivially testable with fake +// constructors. +package registry + +import ( + "context" + "fmt" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider" +) + +// Constructor builds a provider.Provider from its config block. +type Constructor func(ctx context.Context, cfg provider.ProviderConfig) (provider.Provider, error) + +// Build constructs every provider listed in cfg, dispatching on +// ProviderConfig.Type through builtins. Fails fast: an unknown type or a +// constructor error aborts the whole build rather than returning a +// partially-usable provider set that would misbehave once a caller reaches +// for the missing entry. +func Build(ctx context.Context, cfg *provider.Config, builtins map[string]Constructor) (map[string]provider.Provider, error) { + providers := make(map[string]provider.Provider, len(cfg.Providers)) + for _, pc := range cfg.Providers { + ctor, ok := builtins[pc.Type] + if !ok { + return nil, fmt.Errorf("provider %q: unknown type %q", pc.Name, pc.Type) + } + p, err := ctor(ctx, pc) + if err != nil { + return nil, fmt.Errorf("provider %q: %w", pc.Name, err) + } + providers[pc.Name] = p + } + return providers, nil +} diff --git a/internal/provider/registry/registry_test.go b/internal/provider/registry/registry_test.go new file mode 100644 index 0000000..aac3541 --- /dev/null +++ b/internal/provider/registry/registry_test.go @@ -0,0 +1,92 @@ +package registry + +import ( + "context" + "errors" + "testing" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider" +) + +// stubProvider is a minimal provider.Provider satisfying the interface for +// registry tests, which care about wiring, not provider behavior. +type stubProvider struct{ name string } + +func (s *stubProvider) Create(ctx context.Context, req provider.CreateRequest) (string, error) { + return "", nil +} +func (s *stubProvider) Get(ctx context.Context, providerID string) (*provider.Instance, error) { + return nil, provider.ErrNotFound +} +func (s *stubProvider) Delete(ctx context.Context, providerID string) error { return nil } +func (s *stubProvider) ListByTag(ctx context.Context) ([]provider.Instance, error) { + return nil, nil +} + +func stubConstructor(name string) Constructor { + return func(ctx context.Context, cfg provider.ProviderConfig) (provider.Provider, error) { + return &stubProvider{name: name}, nil + } +} + +func TestBuild_dispatchesByType(t *testing.T) { + t.Parallel() + cfg := &provider.Config{Providers: []provider.ProviderConfig{ + {Name: "mock", Type: "mock"}, + {Name: "gcp-eu", Type: "gcp"}, + }} + builtins := map[string]Constructor{ + "mock": stubConstructor("mock"), + "gcp": stubConstructor("gcp"), + } + + providers, err := Build(context.Background(), cfg, builtins) + if err != nil { + t.Fatalf("Build() error = %v, want nil", err) + } + if len(providers) != 2 { + t.Fatalf("len(providers) = %d, want 2", len(providers)) + } + if _, ok := providers["mock"]; !ok { + t.Error(`providers["mock"] missing`) + } + if _, ok := providers["gcp-eu"]; !ok { + t.Error(`providers["gcp-eu"] missing, keyed by config name not type`) + } +} + +func TestBuild_unknownType(t *testing.T) { + t.Parallel() + cfg := &provider.Config{Providers: []provider.ProviderConfig{ + {Name: "p1", Type: "azure"}, + }} + _, err := Build(context.Background(), cfg, map[string]Constructor{"mock": stubConstructor("mock")}) + if err == nil { + t.Fatal("Build() error = nil, want error for unknown type") + } +} + +func TestBuild_constructorErrorAbortsWholeBuild(t *testing.T) { + t.Parallel() + failing := errors.New("boom") + cfg := &provider.Config{Providers: []provider.ProviderConfig{ + {Name: "mock", Type: "mock"}, + {Name: "bad", Type: "bad"}, + }} + builtins := map[string]Constructor{ + "mock": stubConstructor("mock"), + "bad": func(ctx context.Context, cfg provider.ProviderConfig) (provider.Provider, error) { + return nil, failing + }, + } + providers, err := Build(context.Background(), cfg, builtins) + if err == nil { + t.Fatal("Build() error = nil, want error from failing constructor") + } + if !errors.Is(err, failing) { + t.Errorf("Build() error = %v, want it to wrap %v", err, failing) + } + if providers != nil { + t.Errorf("Build() providers = %v, want nil on failure (no partial result)", providers) + } +}