fix: certificate issues

Author: SoulKyu

.planning/phases/49-directory-service-management/CLAUDE.md

@@ -0,0 +1,7 @@
+<claude-mem-context>
+# Recent Activity
+
+<!-- This section is auto-generated by claude-mem. Edit content outside the tags. -->
+
+*No recent activity*
+</claude-mem-context>

.planning/phases/50.1-fix-directory-service-role-post-missing-names-query-param/.gitkeep

@@ -28,6 +28,7 @@ project_name: "terraform-provider-flashblade"
 # Note that when using the JetBrains backend, language servers are not used and this list is correspondingly ignored.
 languages:
 - go
+- python
 
 # the encoding used by text files in the project
 # For a list of possible encodings, see https://docs.python.org/3.11/library/codecs.html#standard-encodings

.serena/project.yml

@@ -34,7 +34,7 @@ resource "flashblade_certificate" "example" {
 
 ### Optional
 
-- `certificate_type` (String) The certificate type (e.g. appliance, external). Defaults to 'appliance' when not specified.
+- `certificate_type` (String) The certificate type. Valid values: 'array' (FlashBlade identity, requires private_key) or 'external' (trusted external server such as AD). When unset, the provider infers 'array' if private_key is provided; otherwise the API defaults to 'external'. Immutable after creation.
 - `intermediate_certificate` (String) The PEM-encoded intermediate certificate chain.
 - `passphrase` (String, Sensitive) The passphrase protecting the private key. Not returned by the API after creation.
 - `private_key` (String, Sensitive) The PEM-encoded private key. Not returned by the API after creation.

docs/resources/certificate.md

@@ -36,7 +36,7 @@ func TestUnit_Certificate_Get_Found(t *testing.T) {
 		ID:              "cert-seed-1",
 		Name:            "my-cert",
 		Certificate:     "-----BEGIN CERTIFICATE-----\nMIItest\n-----END CERTIFICATE-----",
-		CertificateType: "appliance",
+		CertificateType: "array",
 		CommonName:      "test-cert",
 		IssuedBy:        "CN=Test CA",
 		IssuedTo:        "CN=test-cert",
@@ -100,8 +100,9 @@ func TestUnit_Certificate_Post(t *testing.T) {
 
 	pem := "-----BEGIN CERTIFICATE-----\nMIItest\n-----END CERTIFICATE-----"
 	got, err := c.PostCertificate(context.Background(), "new-cert", client.CertificatePost{
-		Certificate: pem,
-		PrivateKey:  "-----BEGIN PRIVATE KEY-----\nMIIkey\n-----END PRIVATE KEY-----",
+		Certificate:     pem,
+		CertificateType: "array",
+		PrivateKey:      "-----BEGIN PRIVATE KEY-----\nMIIkey\n-----END PRIVATE KEY-----",
 	})
 	if err != nil {
 		t.Fatalf("PostCertificate: %v", err)
@@ -118,8 +119,8 @@ func TestUnit_Certificate_Post(t *testing.T) {
 	if got.Status != "imported" {
 		t.Errorf("expected Status %q, got %q", "imported", got.Status)
 	}
-	if got.CertificateType != "appliance" {
-		t.Errorf("expected CertificateType %q, got %q", "appliance", got.CertificateType)
+	if got.CertificateType != "array" {
+		t.Errorf("expected CertificateType %q, got %q", "array", got.CertificateType)
 	}
 	if got.IssuedBy != "CN=Test CA" {
 		t.Errorf("expected IssuedBy %q, got %q", "CN=Test CA", got.IssuedBy)

internal/client/certificates_test.go

@@ -100,7 +100,7 @@ func TestUnit_CertificateDataSource_Basic(t *testing.T) {
 		ID:                      "cert-ds-001",
 		Name:                    "ds-cert",
 		Certificate:             "-----BEGIN CERTIFICATE-----\nMIIBds\n-----END CERTIFICATE-----",
-		CertificateType:         "appliance",
+		CertificateType:         "array",
 		CommonName:              "flashblade.example.com",
 		Country:                 "US",
 		Email:                   "admin@example.com",
@@ -148,8 +148,8 @@ func TestUnit_CertificateDataSource_Basic(t *testing.T) {
 	if model.Name.ValueString() != "ds-cert" {
 		t.Errorf("expected name=ds-cert, got %s", model.Name.ValueString())
 	}
-	if model.CertificateType.ValueString() != "appliance" {
-		t.Errorf("expected certificate_type=appliance, got %s", model.CertificateType.ValueString())
+	if model.CertificateType.ValueString() != "array" {
+		t.Errorf("expected certificate_type=array, got %s", model.CertificateType.ValueString())
 	}
 	if model.CommonName.ValueString() != "flashblade.example.com" {
 		t.Errorf("expected common_name=flashblade.example.com, got %s", model.CommonName.ValueString())

internal/provider/certificate_data_source_test.go

@@ -96,7 +96,7 @@ func (r *certificateResource) Schema(ctx context.Context, _ resource.SchemaReque
 			"certificate_type": schema.StringAttribute{
 				Optional:    true,
 				Computed:    true,
-				Description: "The certificate type (e.g. appliance, external). Defaults to 'appliance' when not specified.",
+				Description: "The certificate type. Valid values: 'array' (FlashBlade identity, requires private_key) or 'external' (trusted external server such as AD). When unset, the provider infers 'array' if private_key is provided; otherwise the API defaults to 'external'. Immutable after creation.",
 				PlanModifiers: []planmodifier.String{
 					stringplanmodifier.UseStateForUnknown(),
 				},
@@ -228,16 +228,28 @@ func (r *certificateResource) Create(ctx context.Context, req resource.CreateReq
 	post := client.CertificatePost{
 		Certificate: data.Certificate.ValueString(),
 	}
-	if !data.CertificateType.IsNull() && !data.CertificateType.IsUnknown() {
+	privateKeySet := !data.PrivateKey.IsNull() && data.PrivateKey.ValueString() != ""
+	switch {
+	case !data.CertificateType.IsNull() && !data.CertificateType.IsUnknown():
 		post.CertificateType = data.CertificateType.ValueString()
-	}
-	if !data.IntermediateCertificate.IsNull() {
+	case privateKeySet:
+		// The FlashBlade API defaults to "external" when certificate_type is
+		// omitted, and rejects private_key for external certificates. Infer
+		// "array" when a private key is supplied so the common case works
+		// without the user needing to set certificate_type explicitly.
+		// Note: the swagger description names this value "appliance", but the
+		// real API only accepts "array" (verified against the Pure-Storage
+		// Ansible FlashBlade collection and live array behavior).
+		post.CertificateType = "array"
+	}
+	wasIntermediateNull := data.IntermediateCertificate.IsNull()
+	if !wasIntermediateNull {
 		post.IntermediateCertificate = data.IntermediateCertificate.ValueString()
 	}
 	if !data.Passphrase.IsNull() {
 		post.Passphrase = data.Passphrase.ValueString()
 	}
-	if !data.PrivateKey.IsNull() {
+	if privateKeySet {
 		post.PrivateKey = data.PrivateKey.ValueString()
 	}
 
@@ -253,6 +265,12 @@ func (r *certificateResource) Create(ctx context.Context, req resource.CreateReq
 
 	mapCertificateToModel(cert, &data)
 
+	// The API returns "" for fields that were never set; Terraform requires the
+	// state to stay null when the config was null (Optional, non-Computed).
+	if wasIntermediateNull && cert.IntermediateCertificate == "" {
+		data.IntermediateCertificate = types.StringNull()
+	}
+
 	data.PrivateKey = privateKey
 	data.Passphrase = passphrase
 
@@ -427,9 +445,14 @@ func (r *certificateResource) Read(ctx context.Context, req resource.ReadRequest
 	// Preserve write-only fields from prior state.
 	privateKey := data.PrivateKey
 	passphrase := data.Passphrase
+	wasIntermediateNull := data.IntermediateCertificate.IsNull()
 
 	mapCertificateToModel(cert, &data)
 
+	if wasIntermediateNull && cert.IntermediateCertificate == "" {
+		data.IntermediateCertificate = types.StringNull()
+	}
+
 	data.PrivateKey = privateKey
 	data.Passphrase = passphrase
 
@@ -490,9 +513,14 @@ func (r *certificateResource) Update(ctx context.Context, req resource.UpdateReq
 		// Preserve write-only fields before mapping overwrites the model.
 		privateKey := plan.PrivateKey
 		passphrase := plan.Passphrase
+		wasIntermediateNull := plan.IntermediateCertificate.IsNull()
 
 		mapCertificateToModel(cert, &plan)
 
+		if wasIntermediateNull && cert.IntermediateCertificate == "" {
+			plan.IntermediateCertificate = types.StringNull()
+		}
+
 		plan.PrivateKey = privateKey
 		plan.Passphrase = passphrase
 	} else {
@@ -558,6 +586,12 @@ func (r *certificateResource) ImportState(ctx context.Context, req resource.Impo
 
 	mapCertificateToModel(cert, &data)
 
+	// intermediate_certificate is Optional (non-Computed): keep it null when the
+	// API reports no intermediate, so a follow-up plan doesn't show bogus drift.
+	if cert.IntermediateCertificate == "" {
+		data.IntermediateCertificate = types.StringNull()
+	}
+
 	// Write-only sensitive fields are not returned by the API — set to empty string after import.
 	data.PrivateKey = types.StringValue("")
 	data.Passphrase = types.StringValue("")

internal/provider/certificate_resource.go

@@ -228,6 +228,117 @@ func TestUnit_CertificateResource_Lifecycle(t *testing.T) {
 	}
 }
 
+// TestUnit_CertificateResource_Create_DefaultsToArrayWhenPrivateKey: verifies that
+// when certificate_type is unset but private_key is provided, the provider sends
+// "array" so the API accepts the request (API otherwise defaults to "external"
+// and rejects the private key).
+func TestUnit_CertificateResource_Create_DefaultsToArrayWhenPrivateKey(t *testing.T) {
+	ms := testmock.NewMockServer()
+	defer ms.Close()
+	handlers.RegisterCertificateHandlers(ms.Mux)
+
+	r := newTestCertificateResource(t, ms)
+	s := certificateResourceSchema(t).Schema
+
+	const certPEM = "-----BEGIN CERTIFICATE-----\nMIIBarr\n-----END CERTIFICATE-----"
+	const privKey = "-----BEGIN PRIVATE KEY-----\nMIIEarr\n-----END PRIVATE KEY-----"
+
+	plan := certificatePlanWith(t, "arr-cert", certPEM, privKey, "")
+	createResp := &resource.CreateResponse{
+		State: tfsdk.State{Raw: tftypes.NewValue(buildCertificateType(), nil), Schema: s},
+	}
+	r.Create(context.Background(), resource.CreateRequest{Plan: plan}, createResp)
+	if createResp.Diagnostics.HasError() {
+		t.Fatalf("Create returned error: %s", createResp.Diagnostics)
+	}
+
+	var got certificateModel
+	if diags := createResp.State.Get(context.Background(), &got); diags.HasError() {
+		t.Fatalf("Get state: %s", diags)
+	}
+	if got.CertificateType.ValueString() != "array" {
+		t.Errorf("expected certificate_type=array (inferred from private_key), got %q", got.CertificateType.ValueString())
+	}
+}
+
+// TestUnit_CertificateResource_Create_ExternalWhenNoPrivateKey: verifies that when
+// neither certificate_type nor private_key is provided, the provider lets the API
+// apply its default ("external").
+func TestUnit_CertificateResource_Create_ExternalWhenNoPrivateKey(t *testing.T) {
+	ms := testmock.NewMockServer()
+	defer ms.Close()
+	handlers.RegisterCertificateHandlers(ms.Mux)
+
+	r := newTestCertificateResource(t, ms)
+	s := certificateResourceSchema(t).Schema
+
+	const certPEM = "-----BEGIN CERTIFICATE-----\nMIIBext\n-----END CERTIFICATE-----"
+
+	plan := certificatePlanWith(t, "ext-cert", certPEM, "", "")
+	createResp := &resource.CreateResponse{
+		State: tfsdk.State{Raw: tftypes.NewValue(buildCertificateType(), nil), Schema: s},
+	}
+	r.Create(context.Background(), resource.CreateRequest{Plan: plan}, createResp)
+	if createResp.Diagnostics.HasError() {
+		t.Fatalf("Create returned error: %s", createResp.Diagnostics)
+	}
+
+	var got certificateModel
+	if diags := createResp.State.Get(context.Background(), &got); diags.HasError() {
+		t.Fatalf("Get state: %s", diags)
+	}
+	if got.CertificateType.ValueString() != "external" {
+		t.Errorf("expected certificate_type=external (API default), got %q", got.CertificateType.ValueString())
+	}
+}
+
+// TestUnit_CertificateResource_Create_PreservesNullIntermediate: verifies that when
+// intermediate_certificate is unset in config and the API returns "", the state
+// remains null rather than "" (avoids "provider produced inconsistent result").
+func TestUnit_CertificateResource_Create_PreservesNullIntermediate(t *testing.T) {
+	ms := testmock.NewMockServer()
+	defer ms.Close()
+	handlers.RegisterCertificateHandlers(ms.Mux)
+
+	r := newTestCertificateResource(t, ms)
+	s := certificateResourceSchema(t).Schema
+
+	const certPEM = "-----BEGIN CERTIFICATE-----\nMIIBnoint\n-----END CERTIFICATE-----"
+
+	plan := certificatePlanWith(t, "noint-cert", certPEM, "", "external")
+	createResp := &resource.CreateResponse{
+		State: tfsdk.State{Raw: tftypes.NewValue(buildCertificateType(), nil), Schema: s},
+	}
+	r.Create(context.Background(), resource.CreateRequest{Plan: plan}, createResp)
+	if createResp.Diagnostics.HasError() {
+		t.Fatalf("Create returned error: %s", createResp.Diagnostics)
+	}
+
+	var got certificateModel
+	if diags := createResp.State.Get(context.Background(), &got); diags.HasError() {
+		t.Fatalf("Get state: %s", diags)
+	}
+	if !got.IntermediateCertificate.IsNull() {
+		t.Errorf("expected intermediate_certificate to stay null, got %q (IsNull=%v)",
+			got.IntermediateCertificate.ValueString(), got.IntermediateCertificate.IsNull())
+	}
+
+	// Read path must also preserve null on empty API response.
+	readResp := &resource.ReadResponse{State: createResp.State}
+	r.Read(context.Background(), resource.ReadRequest{State: createResp.State}, readResp)
+	if readResp.Diagnostics.HasError() {
+		t.Fatalf("Read returned error: %s", readResp.Diagnostics)
+	}
+	var afterRead certificateModel
+	if diags := readResp.State.Get(context.Background(), &afterRead); diags.HasError() {
+		t.Fatalf("Get read state: %s", diags)
+	}
+	if !afterRead.IntermediateCertificate.IsNull() {
+		t.Errorf("expected intermediate_certificate null after Read, got %q",
+			afterRead.IntermediateCertificate.ValueString())
+	}
+}
+
 // TestUnit_CertificateResource_Import: seed cert in mock → import by name → verify state.
 func TestUnit_CertificateResource_Import(t *testing.T) {
 	ms := testmock.NewMockServer()
@@ -242,7 +353,7 @@ func TestUnit_CertificateResource_Import(t *testing.T) {
 		ID:              "cert-import-001",
 		Name:            "import-cert",
 		Certificate:     "-----BEGIN CERTIFICATE-----\nMIIBseed\n-----END CERTIFICATE-----",
-		CertificateType: "appliance",
+		CertificateType: "array",
 		CommonName:      "flashblade.example.com",
 		IssuedBy:        "CN=Test CA",
 		IssuedTo:        "CN=flashblade.example.com",
@@ -273,8 +384,8 @@ func TestUnit_CertificateResource_Import(t *testing.T) {
 	if model.Name.ValueString() != "import-cert" {
 		t.Errorf("expected name=import-cert, got %s", model.Name.ValueString())
 	}
-	if model.CertificateType.ValueString() != "appliance" {
-		t.Errorf("expected certificate_type=appliance, got %s", model.CertificateType.ValueString())
+	if model.CertificateType.ValueString() != "array" {
+		t.Errorf("expected certificate_type=array, got %s", model.CertificateType.ValueString())
 	}
 	if model.CommonName.ValueString() != "flashblade.example.com" {
 		t.Errorf("expected common_name=flashblade.example.com, got %s", model.CommonName.ValueString())

internal/provider/certificate_resource_test.go

@@ -117,9 +117,11 @@ func (s *certificateStore) handlePost(w http.ResponseWriter, r *http.Request) {
 	id := fmt.Sprintf("cert-%d", s.nextID)
 	s.nextID++
 
+	// Match real FlashBlade API: certificate_type defaults to "external" when
+	// omitted at creation time (see swagger _certificateBase.certificate_type).
 	certType := body.CertificateType
 	if certType == "" {
-		certType = "appliance"
+		certType = "external"
 	}
 
 	cert := &client.Certificate{