From ff2dff7af23582c4d2c406f7a409f88486974e88 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sun, 30 Aug 2026 17:05:47 +1000 Subject: [PATCH] Add optional methods attribute to the arrstack role resource (#3) ## Why Engine plugin v0.2.0 added a `methods` field to arrstack roles, pinning a minted arrproxy key to a set of HTTP methods so a read-only integration can be handed a key that cannot write. The provider had no way to express it, so those roles could not be managed from `terraform-vault`. ## How - Adds an optional `methods` set attribute to `arrstack_secret_backend_role`, validated at plan time against `GET/HEAD/POST/PUT/PATCH/DELETE/OPTIONS` (upper case only, since the engine stores them upper-cased and a lower-case value would drift on every plan). - Always writes `methods`: the engine only clears an existing scope when the key is present, so an omitted key would leave a stale scope behind. - Reads an unrestricted role back as null rather than an empty set, so a config that omits `methods` shows no drift; a scope cleared out of band still surfaces as a diff. - Documents the attribute in the README and both example configs. - Tests cover the write mapping (null and populated), the read-back cases (absent/empty/null/cleared), and the plan-time validator. Dependency: engine plugin >= 0.2.0. Reviewed-on: https://git.unkin.net/unkin/terraform-provider-vault-secrets-arrstack/pulls/3 Co-authored-by: unkin-agent Co-committed-by: unkin-agent --- README.md | 18 ++- examples/main.tf | 10 ++ .../arrstack_secret_backend_role/main.tf | 12 ++ go.mod | 1 + go.sum | 2 + .../provider/resource_secret_backend_role.go | 35 ++++ .../resource_secret_backend_role_test.go | 149 ++++++++++++++++++ 7 files changed, 226 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 6e629ad..f7d5751 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ Source address: `artifactapi.k8s.syd1.au.unkin.net/terraform-unkin/vault-secrets | Resource | Manages | |----------|---------| | `arrstack_secret_backend` | Mounts the engine at a path and writes its `config` (arrproxy base URL, request timeout, seeded admin token, optional CA cert). | -| `arrstack_secret_backend_role` | A role: `apps` (subset of sonarr/radarr/prowlarr), `ttl`, `max_ttl`. | +| `arrstack_secret_backend_role` | A role: `apps` (subset of sonarr/radarr/prowlarr), optional `methods` (HTTP method scope), `ttl`, `max_ttl`. | ## Usage @@ -45,6 +45,16 @@ resource "arrstack_secret_backend_role" "all" { ttl = 60 max_ttl = 86400 } + +# Read-only role: keys minted from it may only issue GET/HEAD. +resource "arrstack_secret_backend_role" "sonarr_ro" { + backend = arrstack_secret_backend.arrstack.path + name = "sonarr-ro" + apps = ["sonarr"] + methods = ["GET", "HEAD"] + ttl = 60 + max_ttl = 86400 +} ``` ### Notes @@ -55,6 +65,12 @@ resource "arrstack_secret_backend_role" "all" { - Writing `config` makes the engine authenticate against arrproxy as an admin, so a bad URL or token fails the apply. - `apps` is required; entries must be a subset of `sonarr`, `radarr`, `prowlarr`. +- `methods` is optional and restricts a minted key to those HTTP methods; + omitting it (or setting `[]`) leaves the key unrestricted. Entries must be + upper case and a subset of `GET`, `HEAD`, `POST`, `PUT`, `PATCH`, `DELETE`, + `OPTIONS` — the engine stores them upper-cased, so a lower-case value would + show permanent drift and is rejected at plan time. Requires engine plugin + `vault-plugin-secrets-arrstack` >= 0.2.0. ## Import diff --git a/examples/main.tf b/examples/main.tf index 0fe2292..3c00ee1 100644 --- a/examples/main.tf +++ b/examples/main.tf @@ -39,3 +39,13 @@ resource "arrstack_secret_backend_role" "prowlarr" { ttl = 60 max_ttl = 86400 } + +# Role scoped to read-only traffic across all three apps. +resource "arrstack_secret_backend_role" "readonly" { + backend = arrstack_secret_backend.arrstack.path + name = "readonly" + apps = ["sonarr", "radarr", "prowlarr"] + methods = ["GET", "HEAD"] + ttl = 60 + max_ttl = 86400 +} diff --git a/examples/resources/arrstack_secret_backend_role/main.tf b/examples/resources/arrstack_secret_backend_role/main.tf index ee28976..1324565 100644 --- a/examples/resources/arrstack_secret_backend_role/main.tf +++ b/examples/resources/arrstack_secret_backend_role/main.tf @@ -9,3 +9,15 @@ resource "arrstack_secret_backend_role" "sonarr" { ttl = 60 # 1m max_ttl = 86400 # 24h } + +# The same role narrowed to read-only traffic: keys minted from it may only +# issue GET/HEAD against Sonarr. Omitting methods leaves a key unrestricted. +resource "arrstack_secret_backend_role" "sonarr_ro" { + backend = arrstack_secret_backend.arrstack.path + name = "sonarr-ro" + apps = ["sonarr"] + methods = ["GET", "HEAD"] + + ttl = 60 # 1m + max_ttl = 86400 # 24h +} diff --git a/go.mod b/go.mod index 31da467..6c5dfe4 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.25 require ( github.com/hashicorp/terraform-plugin-framework v1.15.0 + github.com/hashicorp/terraform-plugin-framework-validators v0.16.0 github.com/hashicorp/vault/api v1.15.0 ) diff --git a/go.sum b/go.sum index e5d4aed..1ec95f3 100644 --- a/go.sum +++ b/go.sum @@ -54,6 +54,8 @@ github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= github.com/hashicorp/terraform-plugin-framework v1.15.0 h1:LQ2rsOfmDLxcn5EeIwdXFtr03FVsNktbbBci8cOKdb4= github.com/hashicorp/terraform-plugin-framework v1.15.0/go.mod h1:hxrNI/GY32KPISpWqlCoTLM9JZsGH3CyYlir09bD/fI= +github.com/hashicorp/terraform-plugin-framework-validators v0.16.0 h1:O9QqGoYDzQT7lwTXUsZEtgabeWW96zUBh47Smn2lkFA= +github.com/hashicorp/terraform-plugin-framework-validators v0.16.0/go.mod h1:Bh89/hNmqsEWug4/XWKYBwtnw3tbz5BAy1L1OgvbIaY= github.com/hashicorp/terraform-plugin-go v0.27.0 h1:ujykws/fWIdsi6oTUT5Or4ukvEan4aN9lY+LOxVP8EE= github.com/hashicorp/terraform-plugin-go v0.27.0/go.mod h1:FDa2Bb3uumkTGSkTFpWSOwWJDwA7bf3vdP3ltLDTH6o= github.com/hashicorp/terraform-plugin-log v0.9.0 h1:i7hOA+vdAItN1/7UrfBqBwvYPQ9TFvymaRGZED3FCV0= diff --git a/internal/provider/resource_secret_backend_role.go b/internal/provider/resource_secret_backend_role.go index 47133aa..e50f41c 100644 --- a/internal/provider/resource_secret_backend_role.go +++ b/internal/provider/resource_secret_backend_role.go @@ -4,15 +4,22 @@ import ( "context" "fmt" + "github.com/hashicorp/terraform-plugin-framework-validators/setvalidator" + "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-framework/types" ) +// validMethods mirrors the engine's knownMethods; the engine normalises to +// upper case, so only the upper-case spelling round-trips without drift. +var validMethods = []string{"GET", "HEAD", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"} + var ( _ resource.Resource = &secretBackendRoleResource{} _ resource.ResourceWithImportState = &secretBackendRoleResource{} @@ -26,6 +33,7 @@ type secretBackendRoleModel struct { Backend types.String `tfsdk:"backend"` Name types.String `tfsdk:"name"` Apps types.Set `tfsdk:"apps"` + Methods types.Set `tfsdk:"methods"` TTL types.Int64 `tfsdk:"ttl"` MaxTTL types.Int64 `tfsdk:"max_ttl"` } @@ -61,6 +69,14 @@ func (r *secretBackendRoleResource) Schema(_ context.Context, _ resource.SchemaR ElementType: types.StringType, Required: true, }, + "methods": schema.SetAttribute{ + Description: "HTTP methods a generated key is limited to (subset of GET, HEAD, POST, PUT, PATCH, DELETE, OPTIONS). Omit or leave empty for unrestricted.", + ElementType: types.StringType, + Optional: true, + Validators: []validator.Set{ + setvalidator.ValueStringsAre(stringvalidator.OneOf(validMethods...)), + }, + }, "ttl": schema.Int64Attribute{ Description: "Default lease TTL in seconds for keys generated from this role.", Optional: true, @@ -199,6 +215,14 @@ func roleData(ctx context.Context, m secretBackendRoleModel) (map[string]interfa diags.Append(m.Apps.ElementsAs(ctx, &apps, false)...) data["apps"] = apps } + // Always sent: the engine only clears a method scope when the key is + // present, so an omitted key would leave a previous scope in place. + methods := []string{} + if !m.Methods.IsNull() && !m.Methods.IsUnknown() { + diags.Append(m.Methods.ElementsAs(ctx, &methods, false)...) + } + data["methods"] = methods + if !m.TTL.IsNull() && !m.TTL.IsUnknown() { data["ttl"] = m.TTL.ValueInt64() } @@ -216,6 +240,17 @@ func applyRoleData(m *secretBackendRoleModel, role map[string]interface{}) diag. diags.Append(appDiags...) m.Apps = appSet + // An unrestricted role reads back as an empty list; keep that as null so a + // config that omits methods does not drift against an empty set. + methods := toStringSlice(role["methods"]) + if len(methods) == 0 && (m.Methods.IsNull() || m.Methods.IsUnknown()) { + m.Methods = types.SetNull(types.StringType) + } else { + methodSet, methodDiags := types.SetValueFrom(context.Background(), types.StringType, methods) + diags.Append(methodDiags...) + m.Methods = methodSet + } + if n, ok := toInt64(role["ttl"]); ok && n != 0 { m.TTL = types.Int64Value(n) } else if m.TTL.IsUnknown() { diff --git a/internal/provider/resource_secret_backend_role_test.go b/internal/provider/resource_secret_backend_role_test.go index ce1e5a2..0e4797b 100644 --- a/internal/provider/resource_secret_backend_role_test.go +++ b/internal/provider/resource_secret_backend_role_test.go @@ -3,8 +3,13 @@ package provider import ( "context" "encoding/json" + "sort" "testing" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-framework/types" ) @@ -39,6 +44,49 @@ func TestRoleDataOmitsUnsetTTLs(t *testing.T) { } } +func TestRoleDataSendsEmptyMethodsWhenNull(t *testing.T) { + // The engine only clears a method scope when the key is present, so an + // unset methods must still be written as an empty list. + m := secretBackendRoleModel{ + Apps: setOf(t, "sonarr"), + Methods: types.SetNull(types.StringType), + TTL: types.Int64Null(), + MaxTTL: types.Int64Null(), + } + data, diags := roleData(context.Background(), m) + if diags.HasError() { + t.Fatalf("roleData: %v", diags) + } + methods, ok := data["methods"].([]string) + if !ok { + t.Fatalf("methods = %#v, want an empty []string", data["methods"]) + } + if len(methods) != 0 { + t.Errorf("methods = %v, want empty", methods) + } +} + +func TestRoleDataIncludesMethods(t *testing.T) { + m := secretBackendRoleModel{ + Apps: setOf(t, "sonarr"), + Methods: setOf(t, "GET", "HEAD"), + TTL: types.Int64Null(), + MaxTTL: types.Int64Null(), + } + data, diags := roleData(context.Background(), m) + if diags.HasError() { + t.Fatalf("roleData: %v", diags) + } + methods, ok := data["methods"].([]string) + if !ok || len(methods) != 2 { + t.Fatalf("methods = %#v, want [GET HEAD]", data["methods"]) + } + sort.Strings(methods) + if methods[0] != "GET" || methods[1] != "HEAD" { + t.Errorf("methods = %v, want [GET HEAD]", methods) + } +} + func TestRoleDataIncludesTTLs(t *testing.T) { m := secretBackendRoleModel{ Apps: setOf(t, "prowlarr"), @@ -120,3 +168,104 @@ func TestApplyRoleDataZeroTTLLeavesNull(t *testing.T) { t.Errorf("max_ttl = %v, want null", m.MaxTTL) } } + +func TestApplyRoleDataMapsMethods(t *testing.T) { + // The engine upper-cases and sorts the method scope it stores. + role := map[string]interface{}{ + "apps": []interface{}{"sonarr"}, + "methods": []interface{}{"GET", "HEAD"}, + } + m := secretBackendRoleModel{ + Methods: setOf(t, "HEAD", "GET"), + TTL: types.Int64Null(), + MaxTTL: types.Int64Null(), + } + if diags := applyRoleData(&m, role); diags.HasError() { + t.Fatalf("applyRoleData: %v", diags) + } + if !m.Methods.Equal(setOf(t, "GET", "HEAD")) { + t.Errorf("methods = %v, want {GET HEAD}", m.Methods) + } +} + +func TestApplyRoleDataUnrestrictedMethodsStayNull(t *testing.T) { + // An unrestricted role reads back as an empty/absent list, which must not + // clobber a null config value into an empty set and show permanent drift. + for name, role := range map[string]map[string]interface{}{ + "absent": {"apps": []interface{}{"sonarr"}}, + "empty": {"apps": []interface{}{"sonarr"}, "methods": []interface{}{}}, + "null": {"apps": []interface{}{"sonarr"}, "methods": nil}, + } { + t.Run(name, func(t *testing.T) { + m := secretBackendRoleModel{ + Methods: types.SetNull(types.StringType), + TTL: types.Int64Null(), + MaxTTL: types.Int64Null(), + } + if diags := applyRoleData(&m, role); diags.HasError() { + t.Fatalf("applyRoleData: %v", diags) + } + if !m.Methods.IsNull() { + t.Errorf("methods = %v, want null", m.Methods) + } + }) + } +} + +func TestApplyRoleDataClearedMethodsShowDrift(t *testing.T) { + // A scope removed out of band must land in state as empty, so the next plan + // proposes putting the configured methods back. + role := map[string]interface{}{"apps": []interface{}{"sonarr"}} + m := secretBackendRoleModel{ + Methods: setOf(t, "GET"), + TTL: types.Int64Null(), + MaxTTL: types.Int64Null(), + } + if diags := applyRoleData(&m, role); diags.HasError() { + t.Fatalf("applyRoleData: %v", diags) + } + if m.Methods.IsNull() || len(m.Methods.Elements()) != 0 { + t.Errorf("methods = %v, want an empty set", m.Methods) + } +} + +func TestRoleSchemaMethodsValidation(t *testing.T) { + ctx := context.Background() + var resp resource.SchemaResponse + NewSecretBackendRoleResource().(*secretBackendRoleResource).Schema(ctx, resource.SchemaRequest{}, &resp) + + attr, ok := resp.Schema.Attributes["methods"].(schema.SetAttribute) + if !ok { + t.Fatalf("methods attribute = %#v, want a schema.SetAttribute", resp.Schema.Attributes["methods"]) + } + if attr.IsRequired() || !attr.IsOptional() { + t.Errorf("methods should be optional, not required") + } + if len(attr.Validators) == 0 { + t.Fatal("methods should carry a value validator") + } + + for _, tc := range []struct { + name string + values []string + wantErr bool + }{ + {name: "allowed", values: []string{"GET", "HEAD", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"}}, + {name: "unknown method", values: []string{"GET", "FETCH"}, wantErr: true}, + {name: "lower case", values: []string{"get"}, wantErr: true}, + } { + t.Run(tc.name, func(t *testing.T) { + req := validator.SetRequest{ + Path: path.Root("methods"), + ConfigValue: setOf(t, tc.values...), + } + var vResp validator.SetResponse + for _, v := range attr.Validators { + v.ValidateSet(ctx, req, &vResp) + } + if got := vResp.Diagnostics.HasError(); got != tc.wantErr { + t.Errorf("validation error = %v, want %v (%v)", got, tc.wantErr, vResp.Diagnostics) + } + }) + } +}