From e204fb93eedbb7ce7fe53205d53a287b48f72821 Mon Sep 17 00:00:00 2001 From: Thomas Krampl Date: Mon, 12 Oct 2026 15:15:06 +0200 Subject: [PATCH] feat(valkey): support version and persistence in apply Convert manifest versions to GraphQL enums and forward persistence settings. Omit empty maxMemoryPolicy values from create and update requests. --- internal/apply/README.md | 23 +++-- internal/apply/resource/parse_test.go | 2 +- internal/apply/resource/valkey.go | 53 +++++++----- internal/apply/resource/valkey_test.go | 72 ++++++++++++++++ internal/apply/validate_test.go | 60 +++++++++++-- internal/naisapi/gql/generated.go | 6 +- internal/valkey/valkey.go | 32 ++++++- internal/valkey/valkey_test.go | 114 +++++++++++++++++++++++++ 8 files changed, 321 insertions(+), 41 deletions(-) create mode 100644 internal/apply/resource/valkey_test.go create mode 100644 internal/valkey/valkey_test.go diff --git a/internal/apply/README.md b/internal/apply/README.md index b766ce23..dc1e4d41 100644 --- a/internal/apply/README.md +++ b/internal/apply/README.md @@ -132,22 +132,31 @@ spec: ```yaml version: v1 -kind: Valkey -metadata: - name: my-cache +type: Valkey +name: my-cache spec: tier: SingleNode # SingleNode | HighAvailability memory: "4GB" # 1GB | 4GB | 8GB | 14GB | 28GB | 56GB | 112GB | 200GB + version: "9.1" # required; use a quoted version string maxMemoryPolicy: allkeys-lru # optional; see below databases: 16 # optional notifyKeyspaceEvents: "Ex" # optional - persistence: # optional; parsed but not yet sent to the API + persistence: # optional disabled: false ``` **maxMemoryPolicy values:** `allkeys-lfu`, `allkeys-lru`, `allkeys-random`, `noeviction`, `volatile-lfu`, `volatile-lru`, `volatile-random`, `volatile-ttl` +Omitting `maxMemoryPolicy` uses the API default on creation and leaves the +existing policy unchanged on update. + +Versions are converted to GraphQL enum names (e.g. `"9.1"` becomes `V9_1`), +without a CLI-maintained version mapping. The API must support the chosen version. +`persistence.disabled` controls RDB dumps and backups; setting it to `true` +means all data is lost on restart. Omitting it leaves the API default on creation +and the existing setting on update. + ### OpenSearch ```yaml @@ -194,12 +203,12 @@ binaryData: # optional; values must be base64-encoded ```yaml version: v1 -kind: Valkey -metadata: - name: sessions +type: Valkey +name: sessions spec: tier: HighAvailability memory: "28GB" + version: "9.1" --- version: v1 kind: OpenSearch diff --git a/internal/apply/resource/parse_test.go b/internal/apply/resource/parse_test.go index 5d6b4751..21acbd0a 100644 --- a/internal/apply/resource/parse_test.go +++ b/internal/apply/resource/parse_test.go @@ -51,7 +51,7 @@ spec: Databases: new(16), MaxMemoryPolicy: "allkeys-lru", NotifyKeyspaceEvents: "Ex", - Persistence: &valkeyPersistence{Disabled: false}, + Persistence: &valkeyPersistence{Disabled: new(false)}, } if diff := cmp.Diff(want, spec); diff != "" { t.Errorf("spec mismatch (-want +got):\n%s", diff) diff --git a/internal/apply/resource/valkey.go b/internal/apply/resource/valkey.go index 81278868..8cfae911 100644 --- a/internal/apply/resource/valkey.go +++ b/internal/apply/resource/valkey.go @@ -2,6 +2,7 @@ package resource import ( "context" + "strings" "github.com/nais/cli/internal/naisapi" "github.com/nais/cli/internal/naisapi/gql" @@ -27,12 +28,11 @@ type valkeySpec struct { Databases *int `yaml:"databases"` NotifyKeyspaceEvents string `yaml:"notifyKeyspaceEvents"` Persistence *valkeyPersistence `yaml:"persistence"` + Version string `yaml:"version"` } -// valkeyPersistence is parsed so it does not trip the strict spec decoder, but -// is ignored until the nais-api mutation supports it. type valkeyPersistence struct { - Disabled bool `yaml:"disabled"` + Disabled *bool `yaml:"disabled"` } var ( @@ -68,25 +68,10 @@ func (v valkeyResource) Apply(ctx context.Context, meta Metadata, m Manifest) (A return "", err } - data := &valkey.Valkey{ - Databases: s.Databases, - NotifyKeyspaceEvents: s.NotifyKeyspaceEvents, - Labels: m.Labels, - } - - var err error - if data.Memory, err = enumValue("memory", s.Memory, valkeyMemory); err != nil { - return "", err - } - if data.Tier, err = enumValue("tier", s.Tier, valkeyTier); err != nil { + data, err := s.toValkey(m.Labels) + if err != nil { return "", err } - if s.MaxMemoryPolicy != "" { - if data.MaxMemoryPolicy, err = enumValue("maxMemoryPolicy", s.MaxMemoryPolicy, valkeyMaxMemoryPolicy); err != nil { - return "", err - } - } - // s.Persistence is parsed but intentionally ignored until the API supports it. vmeta := valkey.Metadata{ Name: m.Name, @@ -110,6 +95,34 @@ func (v valkeyResource) Apply(ctx context.Context, meta Metadata, m Manifest) (A return ActionCreated, nil } +func (s valkeySpec) toValkey(labels map[string]string) (*valkey.Valkey, error) { + data := &valkey.Valkey{ + Databases: s.Databases, + NotifyKeyspaceEvents: s.NotifyKeyspaceEvents, + Labels: labels, + } + if s.Version != "" { + data.Version = gql.ValkeyMajorVersion("V" + strings.ReplaceAll(s.Version, ".", "_")) + } + if s.Persistence != nil { + data.PersistenceDisabled = s.Persistence.Disabled + } + + var err error + if data.Memory, err = enumValue("memory", s.Memory, valkeyMemory); err != nil { + return nil, err + } + if data.Tier, err = enumValue("tier", s.Tier, valkeyTier); err != nil { + return nil, err + } + if s.MaxMemoryPolicy != "" { + if data.MaxMemoryPolicy, err = enumValue("maxMemoryPolicy", s.MaxMemoryPolicy, valkeyMaxMemoryPolicy); err != nil { + return nil, err + } + } + return data, nil +} + // exists reports whether a Valkey instance with the given name already exists. func (v valkeyResource) exists(ctx context.Context, meta valkey.Metadata) (bool, error) { _, err := valkey.Get(ctx, meta) diff --git a/internal/apply/resource/valkey_test.go b/internal/apply/resource/valkey_test.go new file mode 100644 index 00000000..0c9449eb --- /dev/null +++ b/internal/apply/resource/valkey_test.go @@ -0,0 +1,72 @@ +package resource + +import ( + "fmt" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/nais/cli/internal/naisapi/gql" + "github.com/nais/cli/internal/valkey" +) + +func TestValkeySpecToValkey(t *testing.T) { + for _, version := range []struct { + raw string + want gql.ValkeyMajorVersion + }{ + {raw: "8.1", want: "V8_1"}, + {raw: "9.0", want: "V9_0"}, + {raw: "9.1", want: "V9_1"}, + {raw: "10.2", want: "V10_2"}, + } { + for _, persistence := range []struct { + name string + yaml string + want *bool + }{ + {name: "omitted"}, + {name: "empty", yaml: "\n persistence: {}"}, + {name: "enabled", yaml: "\n persistence:\n disabled: false", want: new(false)}, + {name: "disabled", yaml: "\n persistence:\n disabled: true", want: new(true)}, + } { + t.Run(version.raw+"/"+persistence.name, func(t *testing.T) { + m, err := ParseManifest(mustDocument(t, fmt.Sprintf(` +version: v1 +type: Valkey +name: my-cache +spec: + memory: 4GB + tier: SingleNode + version: %q + databases: 16 + maxMemoryPolicy: allkeys-lru + notifyKeyspaceEvents: Ex +`, version.raw)+persistence.yaml)) + if err != nil { + t.Fatal(err) + } + var spec valkeySpec + if err := decodeSpec(&m.Spec, &spec); err != nil { + t.Fatal(err) + } + data, err := spec.toValkey(map[string]string{"purpose": "cache"}) + if err != nil { + t.Fatal(err) + } + want := &valkey.Valkey{ + Memory: gql.ValkeyMemoryGb4, + Tier: gql.ValkeyTierSingleNode, + Version: version.want, + PersistenceDisabled: persistence.want, + Databases: new(16), + MaxMemoryPolicy: gql.ValkeyMaxMemoryPolicyAllkeysLru, + NotifyKeyspaceEvents: "Ex", + Labels: map[string]string{"purpose": "cache"}, + } + if diff := cmp.Diff(want, data); diff != "" { + t.Errorf("Valkey mismatch (-want +got):\n%s", diff) + } + }) + } + } +} diff --git a/internal/apply/validate_test.go b/internal/apply/validate_test.go index 6987185f..db1968f0 100644 --- a/internal/apply/validate_test.go +++ b/internal/apply/validate_test.go @@ -32,10 +32,16 @@ const testResourceSchema = `{ "spec": { "type": "object", "additionalProperties": false, - "required": ["memory", "tier"], + "required": ["memory", "tier", "version"], "properties": { "memory": {"enum": ["1GB", "4GB"]}, - "tier": {"enum": ["SingleNode", "HighAvailability"]} + "tier": {"enum": ["SingleNode", "HighAvailability"]}, + "version": {"type": "string", "enum": ["8.1", "9.0", "9.1"]}, + "persistence": { + "type": "object", + "additionalProperties": false, + "properties": {"disabled": {"type": "boolean"}} + } } } } @@ -122,6 +128,7 @@ name: my-valkey spec: memory: 1GB tier: SingleNode + version: "9.1" `) if err != nil { t.Fatalf("Run: %v\noutput:\n%s", err, out) @@ -141,10 +148,39 @@ name: my-valkey spec: memory: 999GB tier: SingleNode + version: "9.1" `) mustErrorContains(t, err, "Valkey/my-valkey") } +func TestRun_DryRunValidatesValkeyVersion(t *testing.T) { + withTestResourceSchema(t, gojsonschema.NewStringLoader(testResourceSchema)) + + for _, version := range []string{`"8.1"`, `"9.0"`, `"9.1"`, `"10.2"`, `9.1`, ""} { + t.Run(version, func(t *testing.T) { + field := "" + if version != "" { + field = "\n version: " + version + } + out, err := writeManifestAndDryRun(t, ` +version: v1 +type: Valkey +name: my-valkey +spec: + memory: 1GB + tier: SingleNode`+field) + switch version { + case `"8.1"`, `"9.0"`, `"9.1"`: + if err != nil { + t.Fatalf("Run: %v\noutput:\n%s", err, out) + } + default: + mustErrorContains(t, err, "version") + } + }) + } +} + func TestRun_DryRunRejectsUnknownNativeValkeySpecField(t *testing.T) { withTestResourceSchema(t, gojsonschema.NewStringLoader(testResourceSchema)) @@ -155,6 +191,7 @@ name: my-valkey spec: memory: 1GB tier: SingleNode + version: "9.1" bogusField: nope `) mustErrorContains(t, err, "Valkey/my-valkey") @@ -171,6 +208,7 @@ bogusTopLevel: nope spec: memory: 1GB tier: SingleNode + version: "9.1" `) mustErrorContains(t, err, "Valkey/my-valkey") mustErrorContains(t, err, "bogusTopLevel") @@ -256,6 +294,7 @@ name: first spec: memory: 1GB tier: SingleNode + version: "9.1" --- version: v1 type: Postgres @@ -282,6 +321,7 @@ name: first spec: memory: 1GB tier: SingleNode + version: "9.1" --- version: v1 type: Valkey @@ -290,6 +330,7 @@ namespace: ignored spec: memory: 1GB tier: SingleNode + version: "9.1" `, dryRun, false) mustErrorContains(t, err, "Valkey/second contains fields not used by nais apply: namespace") mustErrorContains(t, err, "--allow-ignored-fields") @@ -299,21 +340,23 @@ spec: } } -func TestRun_RejectsIgnoredValkeyPersistence(t *testing.T) { +func TestRun_DryRunAcceptsValkeyPersistence(t *testing.T) { withTestResourceSchema(t, gojsonschema.NewStringLoader(testResourceSchema)) - for _, dryRun := range []bool{false, true} { - _, err := writeManifestAndRun(t, ` + for _, disabled := range []string{"true", "false"} { + out, err := writeManifestAndDryRun(t, ` version: v1 type: Valkey name: my-valkey spec: memory: 1GB tier: SingleNode + version: "9.1" persistence: - disabled: true -`, dryRun, false) - mustErrorContains(t, err, "persistence") + disabled: `+disabled) + if err != nil { + t.Fatalf("Run: %v\noutput:\n%s", err, out) + } } } @@ -328,6 +371,7 @@ namespace: ignored spec: memory: 1GB tier: SingleNode + version: "9.1" `, true, true) if err != nil { t.Fatalf("Run: %v\noutput:\n%s", err, out) diff --git a/internal/naisapi/gql/generated.go b/internal/naisapi/gql/generated.go index b2c3541f..ce32ab42 100644 --- a/internal/naisapi/gql/generated.go +++ b/internal/naisapi/gql/generated.go @@ -1506,7 +1506,7 @@ type CreateValkeyInput struct { MaxMemoryPolicy *ValkeyMaxMemoryPolicy `json:"maxMemoryPolicy,omitempty"` NotifyKeyspaceEvents *string `json:"notifyKeyspaceEvents,omitempty"` Databases *int `json:"databases,omitempty"` - PersistenceDisabled *bool `json:"persistenceDisabled"` + PersistenceDisabled *bool `json:"persistenceDisabled,omitempty"` } // GetName returns CreateValkeyInput.Name, and is useful for accessing the field via an interface. @@ -36679,12 +36679,12 @@ type UpdateValkeyInput struct { TeamSlug string `json:"teamSlug"` Tier ValkeyTier `json:"tier"` Memory ValkeyMemory `json:"memory"` - Version *ValkeyMajorVersion `json:"version"` + Version *ValkeyMajorVersion `json:"version,omitempty"` MaxMemoryPolicy *ValkeyMaxMemoryPolicy `json:"maxMemoryPolicy,omitempty"` NotifyKeyspaceEvents *string `json:"notifyKeyspaceEvents,omitempty"` Databases *int `json:"databases,omitempty"` Labels []ResourceLabelInput `json:"labels"` - PersistenceDisabled *bool `json:"persistenceDisabled"` + PersistenceDisabled *bool `json:"persistenceDisabled,omitempty"` } // GetName returns UpdateValkeyInput.Name, and is useful for accessing the field via an interface. diff --git a/internal/valkey/valkey.go b/internal/valkey/valkey.go index 21cd5524..736d6634 100644 --- a/internal/valkey/valkey.go +++ b/internal/valkey/valkey.go @@ -20,6 +20,9 @@ type Valkey struct { // NotifyKeyspaceEvents configures Valkey keyspace notifications, e.g. "Ex". NotifyKeyspaceEvents string `json:"notifyKeyspaceEvents,omitempty" toml:"notifyKeyspaceEvents,omitempty"` + Version gql.ValkeyMajorVersion `json:"version,omitempty" toml:"version,omitempty"` + PersistenceDisabled *bool `json:"persistenceDisabled,omitempty" toml:"persistenceDisabled,omitempty"` + Labels map[string]string `json:"labels,omitempty" toml:"labels,omitempty"` } @@ -37,6 +40,7 @@ func Create(ctx context.Context, metadata Metadata, data *Valkey) (*gql.CreateVa # @genqlient(for: "CreateValkeyInput.maxMemoryPolicy", omitempty: true) # @genqlient(for: "CreateValkeyInput.notifyKeyspaceEvents", omitempty: true) # @genqlient(for: "CreateValkeyInput.databases", omitempty: true) + # @genqlient(for: "CreateValkeyInput.persistenceDisabled", omitempty: true) mutation CreateValkey( $input: CreateValkeyInput! ) { @@ -54,13 +58,24 @@ func Create(ctx context.Context, metadata Metadata, data *Valkey) (*gql.CreateVa return nil, err } + version := data.Version + if version == "" { + version = gql.ValkeyMajorVersionV91 + } + var maxMemoryPolicy *gql.ValkeyMaxMemoryPolicy + if data.MaxMemoryPolicy != "" { + maxMemoryPolicy = new(data.MaxMemoryPolicy) + } + resp, err := gql.CreateValkey(ctx, client, gql.CreateValkeyInput{ Name: metadata.Name, EnvironmentName: metadata.EnvironmentName, TeamSlug: metadata.TeamSlug, Memory: data.Memory, Tier: data.Tier, - MaxMemoryPolicy: new(data.MaxMemoryPolicy), + Version: version, + PersistenceDisabled: data.PersistenceDisabled, + MaxMemoryPolicy: maxMemoryPolicy, NotifyKeyspaceEvents: new(data.NotifyKeyspaceEvents), Databases: data.Databases, }) @@ -185,6 +200,8 @@ func Update(ctx context.Context, metadata Metadata, data *Valkey) (*gql.UpdateVa # @genqlient(for: "UpdateValkeyInput.maxMemoryPolicy", omitempty: true) # @genqlient(for: "UpdateValkeyInput.notifyKeyspaceEvents", omitempty: true) # @genqlient(for: "UpdateValkeyInput.databases", omitempty: true) + # @genqlient(for: "UpdateValkeyInput.version", omitempty: true) + # @genqlient(for: "UpdateValkeyInput.persistenceDisabled", omitempty: true) mutation UpdateValkey( $input: UpdateValkeyInput! ) { @@ -210,13 +227,24 @@ func Update(ctx context.Context, metadata Metadata, data *Valkey) (*gql.UpdateVa }) } + var version *gql.ValkeyMajorVersion + if data.Version != "" { + version = new(data.Version) + } + var maxMemoryPolicy *gql.ValkeyMaxMemoryPolicy + if data.MaxMemoryPolicy != "" { + maxMemoryPolicy = new(data.MaxMemoryPolicy) + } + resp, err := gql.UpdateValkey(ctx, client, gql.UpdateValkeyInput{ Name: metadata.Name, EnvironmentName: metadata.EnvironmentName, TeamSlug: metadata.TeamSlug, Memory: data.Memory, Tier: data.Tier, - MaxMemoryPolicy: new(data.MaxMemoryPolicy), + Version: version, + PersistenceDisabled: data.PersistenceDisabled, + MaxMemoryPolicy: maxMemoryPolicy, NotifyKeyspaceEvents: new(data.NotifyKeyspaceEvents), Databases: data.Databases, Labels: labels, diff --git a/internal/valkey/valkey_test.go b/internal/valkey/valkey_test.go new file mode 100644 index 00000000..c7312c38 --- /dev/null +++ b/internal/valkey/valkey_test.go @@ -0,0 +1,114 @@ +package valkey + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "strings" + "testing" + + "github.com/nais/cli/internal/naisapi/gql" +) + +type testTransport func(*http.Request) (*http.Response, error) + +func (f testTransport) RoundTrip(r *http.Request) (*http.Response, error) { + return f(r) +} + +func TestValkeyMutationOptionalFields(t *testing.T) { + t.Setenv("NAIS_API_LOCAL_HOST", "api.nais.localhost") + t.Setenv("NAIS_API_LOCAL_EMAIL", "") + original := http.DefaultTransport + t.Cleanup(func() { http.DefaultTransport = original }) + + for _, update := range []bool{false, true} { + for _, tc := range []struct { + name string + version gql.ValkeyMajorVersion + persistence *bool + policy gql.ValkeyMaxMemoryPolicy + }{ + {name: "omitted"}, + {name: "enabled", version: gql.ValkeyMajorVersionV81, persistence: new(false)}, + {name: "disabled", version: gql.ValkeyMajorVersionV91, persistence: new(true)}, + {name: "best-effort", version: "V9_0"}, + {name: "max-memory-policy", policy: gql.ValkeyMaxMemoryPolicyAllkeysLru}, + {name: "no-eviction", policy: gql.ValkeyMaxMemoryPolicyNoEviction}, + } { + t.Run(fmt.Sprintf("update=%t/%s", update, tc.name), func(t *testing.T) { + calls := 0 + http.DefaultTransport = testTransport(func(r *http.Request) (*http.Response, error) { + calls++ + var req struct { + Variables struct { + Input map[string]json.RawMessage `json:"input"` + } `json:"variables"` + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + return nil, err + } + input := req.Variables.Input + gotPolicy, present := input["maxMemoryPolicy"] + if tc.policy == "" { + if present { + t.Errorf("maxMemoryPolicy should be omitted, got %s", gotPolicy) + } + } else if string(gotPolicy) != fmt.Sprintf("%q", tc.policy) { + t.Errorf("maxMemoryPolicy = %s, want %q", gotPolicy, tc.policy) + } + wantVersion := tc.version + if !update && wantVersion == "" { + wantVersion = gql.ValkeyMajorVersionV91 + } + gotVersion, present := input["version"] + if wantVersion == "" { + if present { + t.Errorf("version should be omitted, got %s", gotVersion) + } + } else if string(gotVersion) != fmt.Sprintf("%q", wantVersion) { + t.Errorf("version = %s, want %q", gotVersion, wantVersion) + } + gotPersistence, present := input["persistenceDisabled"] + if tc.persistence == nil { + if present { + t.Errorf("persistenceDisabled should be omitted, got %s", gotPersistence) + } + } else if string(gotPersistence) != fmt.Sprint(*tc.persistence) { + t.Errorf("persistenceDisabled = %s, want %t", gotPersistence, *tc.persistence) + } + field := "createValkey" + if update { + field = "updateValkey" + } + return &http.Response{ + StatusCode: http.StatusOK, + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader(fmt.Sprintf( + `{"data":{"%s":{"valkey":{"id":"id","name":"my-cache"}}}}`, field))), + }, nil + }) + data := &Valkey{ + Memory: gql.ValkeyMemoryGb4, Tier: gql.ValkeyTierSingleNode, + Version: tc.version, PersistenceDisabled: tc.persistence, + MaxMemoryPolicy: tc.policy, + } + meta := Metadata{Name: "my-cache", TeamSlug: "my-team", EnvironmentName: "dev"} + var err error + if update { + _, err = Update(context.Background(), meta, data) + } else { + _, err = Create(context.Background(), meta, data) + } + if err != nil { + t.Fatal(err) + } + if calls != 1 { + t.Errorf("request count = %d, want 1", calls) + } + }) + } + } +}