feat(coderd_ai_provider): delete the stored API key with an empty api_key_wo

docs/resources/ai_provider.md

@@ -102,7 +102,7 @@ resource "coderd_ai_provider" "openai" {
 
 > **NOTE**: [Write-only arguments](https://developer.hashicorp.com/terraform/language/resources/ephemeral#write-only-arguments) are supported in Terraform 1.11 and later.
 
-- `api_key_wo` (String, Sensitive, [Write-only](https://developer.hashicorp.com/terraform/language/resources/ephemeral#write-only-arguments)) Plaintext API key for the provider. Not valid for `bedrock` or `copilot`, or when `settings.bedrock` is set. Optional with `settings.claude_platform_aws`; see that attribute. Removing it from configuration does not delete a stored key; replace the resource to delete it. Bump `api_key_wo_version` to rotate it.
+- `api_key_wo` (String, Sensitive, [Write-only](https://developer.hashicorp.com/terraform/language/resources/ephemeral#write-only-arguments)) Plaintext API key for the provider. Not valid for `bedrock` or `copilot`, or when `settings.bedrock` is set. Optional with `settings.claude_platform_aws`; see that attribute. Removing it from configuration keeps the stored key. Bump `api_key_wo_version` to rotate it, or set it to `""` and bump the version to delete the stored key.
 - `api_key_wo_version` (Number) Version for the write-only API key. Required when `api_key_wo` is set; bump it whenever `api_key_wo` changes to rotate the stored key.
 - `display_name` (String) Display name shown in Coder. If omitted, defaults to the provider name.
 - `enabled` (Boolean) Whether this AI provider is enabled. Defaults to true.

internal/provider/ai_provider_resource.go

@@ -91,6 +91,33 @@ func (r *AIProviderResource) ModifyPlan(ctx context.Context, req resource.Modify
 		"Experimental Resource",
 		"coderd_ai_provider is experimental. Changes are expected, and it is not recommended for production use.",
 	)
+	if req.State.Raw.IsNull() {
+		return
+	}
+
+	var version, priorVersion types.Int64
+	var key types.String
+	resp.Diagnostics.Append(req.Plan.GetAttribute(ctx, path.Root("api_key_wo_version"), &version)...)
+	resp.Diagnostics.Append(req.State.GetAttribute(ctx, path.Root("api_key_wo_version"), &priorVersion)...)
+	resp.Diagnostics.Append(req.Config.GetAttribute(ctx, path.Root("api_key_wo"), &key)...)
+	if resp.Diagnostics.HasError() || version.IsNull() || version.IsUnknown() || version.Equal(priorVersion) || key.IsUnknown() {
+		return
+	}
+	if key.IsNull() {
+		addMissingAPIKeyError(&resp.Diagnostics)
+		return
+	}
+	if key.ValueString() == "" {
+		resp.Diagnostics.AddAttributeWarning(
+			path.Root("api_key_wo"),
+			"Deleting Stored API Key",
+			"`api_key_wo` is `\"\"` and `api_key_wo_version` changed, so this apply deletes the stored API key. The plan shows this only as a version change.",
+		)
+	}
+}
+
+func addMissingAPIKeyError(diags *diag.Diagnostics) {
+	diags.AddAttributeError(path.Root("api_key_wo"), "Missing API Key", "`api_key_wo` must be configured when `api_key_wo_version` changes. Set it to `\"\"` to delete the stored key.")
 }
 
 func (r *AIProviderResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) {
@@ -159,12 +186,11 @@ func (r *AIProviderResource) Schema(ctx context.Context, req resource.SchemaRequ
 				Required:            true,
 			},
 			"api_key_wo": schema.StringAttribute{
-				MarkdownDescription: "Plaintext API key for the provider. Not valid for `bedrock` or `copilot`, or when `settings.bedrock` is set. Optional with `settings.claude_platform_aws`; see that attribute. Removing it from configuration does not delete a stored key; replace the resource to delete it. Bump `api_key_wo_version` to rotate it.",
+				MarkdownDescription: "Plaintext API key for the provider. Not valid for `bedrock` or `copilot`, or when `settings.bedrock` is set. Optional with `settings.claude_platform_aws`; see that attribute. Removing it from configuration keeps the stored key. Bump `api_key_wo_version` to rotate it, or set it to `\"\"` and bump the version to delete the stored key.",
 				Optional:            true,
 				Sensitive:           true,
 				WriteOnly:           true,
 				Validators: []validator.String{
-					stringvalidator.LengthAtLeast(1),
 					stringvalidator.AlsoRequires(path.MatchRoot("api_key_wo_version")),
 				},
 			},
@@ -581,8 +607,8 @@ func (r *AIProviderResource) ImportState(ctx context.Context, req resource.Impor
 
 func (m AIProviderResourceModel) createRequest(config AIProviderResourceModel, diags *diag.Diagnostics) codersdk.CreateAIProviderRequest {
 	var apiKeys []string
-	if !config.APIKeyWO.IsNull() && !config.APIKeyWO.IsUnknown() {
-		apiKeys = []string{config.APIKeyWO.ValueString()}
+	if key := config.APIKeyWO.ValueString(); key != "" {
+		apiKeys = []string{key}
 	}
 	return codersdk.CreateAIProviderRequest{
 		Type:        codersdk.AIProviderType(m.Type.ValueString()),
@@ -618,11 +644,16 @@ func (m AIProviderResourceModel) updateRequest(state, config AIProviderResourceM
 	}
 
 	// Rotate the API key only when its version changes to a concrete value. A
-	// null version preserves the stored key rather than clearing it.
+	// null version preserves the stored key rather than clearing it; an empty
+	// key deletes it.
 	if !m.APIKeyWOVersion.IsNull() && !m.APIKeyWOVersion.Equal(state.APIKeyWOVersion) {
-		if config.APIKeyWO.IsNull() || config.APIKeyWO.IsUnknown() {
-			diags.AddAttributeError(path.Root("api_key_wo"), "Missing API Key", "`api_key_wo` must be configured when `api_key_wo_version` changes.")
-		} else {
+		switch {
+		case config.APIKeyWO.IsNull() || config.APIKeyWO.IsUnknown():
+			// ModifyPlan reports this at plan time unless the version was unknown then.
+			addMissingAPIKeyError(diags)
+		case config.APIKeyWO.ValueString() == "":
+			patch.APIKeys = &[]codersdk.AIProviderKeyMutation{}
+		default:
 			patch.APIKeys = &[]codersdk.AIProviderKeyMutation{{APIKey: stringPtrOrNil(config.APIKeyWO)}}
 		}
 	}

internal/provider/ai_provider_resource_test.go

@@ -57,6 +57,9 @@ func TestAccAIProviderResource(t *testing.T) {
 	// Changing base_url to a different region must re-derive settings.bedrock.region.
 	cfg3 := cfg2
 	cfg3.BedrockBaseURL = "https://bedrock-runtime.us-west-2.amazonaws.com"
+	cfg4 := cfg3
+	cfg4.OpenAIKey = ""
+	cfg4.OpenAIKeyVersion = 3
 
 	resource.Test(t, resource.TestCase{
 		IsUnitTest:               true,
@@ -93,6 +96,10 @@ func TestAccAIProviderResource(t *testing.T) {
 					resource.TestCheckResourceAttr("coderd_ai_provider.bedrock", "settings.bedrock.region", "us-west-2"),
 				),
 			},
+			{
+				Config: cfg4.String(t),
+				Check:  resource.TestCheckNoResourceAttr("coderd_ai_provider.openai", "api_key_masked"),
+			},
 		},
 	})
 }
@@ -254,17 +261,6 @@ func TestAIProviderResourceSchemaValidation(t *testing.T) {
 `,
 			wantError: `api_key_wo_version`,
 		},
-		"api key cannot be empty": {
-			body: `resource "coderd_ai_provider" "test" {
-  type               = "openai"
-  name               = "openai-test"
-  base_url           = "https://api.openai.com/v1"
-  api_key_wo         = ""
-  api_key_wo_version = 1
-}
-`,
-			wantError: `(?s)string length must be at\s+least\s+1`,
-		},
 		"bedrock known config requires region or credentials": {
 			body: `resource "coderd_ai_provider" "test" {
   type     = "bedrock"
@@ -760,6 +756,26 @@ func TestAIProviderCreateRequestBedrockWithoutCredentials(t *testing.T) {
 	require.Nil(t, req.Settings.Bedrock.AccessKeySecret)
 }
 
+func TestAIProviderCreateRequestEmptyAPIKey(t *testing.T) {
+	t.Parallel()
+
+	plan := AIProviderResourceModel{
+		Type:            types.StringValue(string(codersdk.AIProviderTypeOpenAI)),
+		Name:            types.StringValue("openai"),
+		Enabled:         types.BoolValue(true),
+		BaseURL:         types.StringValue("https://api.openai.com/v1"),
+		APIKeyWOVersion: types.Int64Value(1),
+	}
+	config := plan
+	config.APIKeyWO = types.StringValue("")
+
+	var diags diag.Diagnostics
+	req := plan.createRequest(config, &diags)
+	require.False(t, diags.HasError(), diags.Errors())
+	require.Empty(t, req.APIKeys)
+	require.Empty(t, req.Validate())
+}
+
 func TestAIProviderCreateRequestBedrockMantleProtocol(t *testing.T) {
 	t.Parallel()
 
@@ -907,6 +923,67 @@ func TestAIProviderUpdateRequestAPIKeyRotation(t *testing.T) {
 	})
 }
 
+func TestAIProviderModifyPlanAPIKey(t *testing.T) {
+	t.Parallel()
+
+	ctx := t.Context()
+	r := &AIProviderResource{}
+	schemaResp := &fwresource.SchemaResponse{}
+	r.Schema(ctx, fwresource.SchemaRequest{}, schemaResp)
+	require.Empty(t, schemaResp.Diagnostics)
+	s := schemaResp.Schema
+
+	raw := func(t *testing.T, m *AIProviderResourceModel) tftypes.Value {
+		state := tfsdk.State{Schema: s, Raw: tftypes.NewValue(s.Type().TerraformType(ctx), nil)}
+		if m != nil {
+			require.Empty(t, state.Set(ctx, m))
+		}
+		return state.Raw
+	}
+	model := func(version types.Int64, key types.String) *AIProviderResourceModel {
+		return &AIProviderResourceModel{APIKeyWOVersion: version, APIKeyWO: key}
+	}
+	prior := model(types.Int64Value(1), types.StringNull())
+
+	for _, tc := range []struct {
+		name         string
+		state        *AIProviderResourceModel
+		config       *AIProviderResourceModel
+		wantErrors   []string
+		wantWarnings []string
+	}{
+		{name: "version bumped without key", state: prior, config: model(types.Int64Value(2), types.StringNull()), wantErrors: []string{"Missing API Key"}},
+		{name: "version bumped with empty key", state: prior, config: model(types.Int64Value(2), types.StringValue("")), wantWarnings: []string{"Deleting Stored API Key"}},
+		{name: "version bumped with key", state: prior, config: model(types.Int64Value(2), types.StringValue("sk-rotated"))},
+		{name: "version unchanged without key", state: prior, config: model(types.Int64Value(1), types.StringNull())},
+		{name: "version unknown without key", state: prior, config: model(types.Int64Unknown(), types.StringNull())},
+		{name: "create with empty key", config: model(types.Int64Value(1), types.StringValue(""))},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			// Write-only values are null in the plan; ModifyPlan must read them from config.
+			planned := *tc.config
+			planned.APIKeyWO = types.StringNull()
+			plan := tfsdk.Plan{Schema: s, Raw: raw(t, &planned)}
+			resp := &fwresource.ModifyPlanResponse{Plan: plan}
+			r.ModifyPlan(ctx, fwresource.ModifyPlanRequest{
+				Config: tfsdk.Config{Schema: s, Raw: raw(t, tc.config)},
+				Plan:   plan,
+				State:  tfsdk.State{Schema: s, Raw: raw(t, tc.state)},
+			}, resp)
+
+			var errs, warnings []string
+			for _, d := range resp.Diagnostics.Errors() {
+				errs = append(errs, d.Summary())
+			}
+			for _, d := range resp.Diagnostics.Warnings() {
+				warnings = append(warnings, d.Summary())
+			}
+			require.Equal(t, tc.wantErrors, errs)
+			require.Equal(t, append([]string{"Experimental Resource"}, tc.wantWarnings...), warnings)
+		})
+	}
+}
+
 func TestAIProviderUpdateRejectsBedrockCredentialBumpWithoutCredentials(t *testing.T) {
 	t.Parallel()