Skip to content

Commit bf7b16b

Browse files
santoshhashicorpsantoshhashicorp
committed
Addressed PR reivew comments
1 parent 63c1245 commit bf7b16b

2 files changed

Lines changed: 119 additions & 73 deletions

File tree

‎internal/vault/keymgmt/resource_keymgmt_distribute_key.go‎

Lines changed: 82 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
1818
"github.com/hashicorp/terraform-plugin-framework/types"
1919
"github.com/hashicorp/terraform-plugin-log/tflog"
20+
vaultapi "github.com/hashicorp/vault/api"
2021

2122
"github.com/hashicorp/terraform-provider-vault/internal/consts"
2223
"github.com/hashicorp/terraform-provider-vault/internal/framework/base"
@@ -108,17 +109,12 @@ func (r *DistributeKeyResource) Create(ctx context.Context, req resource.CreateR
108109
return
109110
}
110111

111-
cli, err := client.GetClient(ctx, r.Meta(), data.Namespace.ValueString())
112-
if err != nil {
113-
resp.Diagnostics.AddError(errutil.ClientConfigureErr(err))
112+
cli, ok := r.getVaultClient(ctx, data.Namespace.ValueString(), &resp.Diagnostics)
113+
if !ok {
114114
return
115115
}
116116

117-
vaultPath := data.Mount.ValueString()
118-
kmsName := data.KMSName.ValueString()
119-
keyName := data.KeyName.ValueString()
120-
apiPath := BuildDistributeKeyPath(vaultPath, kmsName, keyName)
121-
117+
apiPath := data.APIPath()
122118
writeData := map[string]interface{}{}
123119

124120
var purposes []string
@@ -132,23 +128,27 @@ func (r *DistributeKeyResource) Create(ctx context.Context, req resource.CreateR
132128
writeData["protection"] = data.Protection.ValueString()
133129
}
134130

135-
writeResp, err := cli.Logical().WriteWithContext(ctx, apiPath, writeData)
136-
if err != nil {
131+
if _, err := cli.Logical().WriteWithContext(ctx, apiPath, writeData); err != nil {
137132
resp.Diagnostics.AddError(ErrCreating(ResourceTypeKeyDistribution, apiPath, err))
138133
return
139134
}
140135

141-
// Parse the write response to extract all fields including computed ones
142-
if writeResp != nil && writeResp.Data != nil {
143-
parseDistributeKeyResponse(ctx, writeResp.Data, &data, &resp.Diagnostics)
144-
if resp.Diagnostics.HasError() {
145-
return
146-
}
147-
} else {
148-
// Ensure computed field is set to a known value even if write response is empty
149-
if data.Versions.IsUnknown() {
150-
data.Versions, _ = types.MapValueFrom(ctx, types.StringType, map[string]string{})
151-
}
136+
// Read back the state from Vault
137+
responseData, exists := r.readDistributeKey(ctx, cli, apiPath, &resp.Diagnostics)
138+
if resp.Diagnostics.HasError() {
139+
return
140+
}
141+
if !exists {
142+
resp.Diagnostics.AddError(
143+
"Unexpected error after creating key distribution",
144+
fmt.Sprintf("Key distribution not found at path %q immediately after creation", apiPath),
145+
)
146+
return
147+
}
148+
149+
data.parseDistributeKeyResponse(ctx, responseData, &resp.Diagnostics)
150+
if resp.Diagnostics.HasError() {
151+
return
152152
}
153153

154154
resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
@@ -161,21 +161,17 @@ func (r *DistributeKeyResource) Read(ctx context.Context, req resource.ReadReque
161161
return
162162
}
163163

164-
cli, err := client.GetClient(ctx, r.Meta(), data.Namespace.ValueString())
165-
if err != nil {
166-
resp.Diagnostics.AddError(errutil.ClientConfigureErr(err))
164+
cli, ok := r.getVaultClient(ctx, data.Namespace.ValueString(), &resp.Diagnostics)
165+
if !ok {
167166
return
168167
}
169168

170-
// Build API path from data fields
171-
apiPath := BuildDistributeKeyPath(data.Mount.ValueString(), data.KMSName.ValueString(), data.KeyName.ValueString())
172-
vaultResp, err := cli.Logical().ReadWithContext(ctx, apiPath)
173-
if err != nil {
174-
resp.Diagnostics.AddError(ErrReading(ResourceTypeKeyDistribution, apiPath, err))
169+
apiPath := data.APIPath()
170+
responseData, exists := r.readDistributeKey(ctx, cli, apiPath, &resp.Diagnostics)
171+
if resp.Diagnostics.HasError() {
175172
return
176173
}
177-
178-
if vaultResp == nil {
174+
if !exists {
179175
tflog.Warn(ctx, "key distribution not found, removing from state", map[string]interface{}{
180176
"path": apiPath,
181177
})
@@ -184,7 +180,7 @@ func (r *DistributeKeyResource) Read(ctx context.Context, req resource.ReadReque
184180
}
185181

186182
// Parse response data
187-
parseDistributeKeyResponse(ctx, vaultResp.Data, &data, &resp.Diagnostics)
183+
data.parseDistributeKeyResponse(ctx, responseData, &resp.Diagnostics)
188184
if resp.Diagnostics.HasError() {
189185
return
190186
}
@@ -200,14 +196,12 @@ func (r *DistributeKeyResource) Update(ctx context.Context, req resource.UpdateR
200196
return
201197
}
202198

203-
cli, err := client.GetClient(ctx, r.Meta(), plan.Namespace.ValueString())
204-
if err != nil {
205-
resp.Diagnostics.AddError(errutil.ClientConfigureErr(err))
199+
cli, ok := r.getVaultClient(ctx, plan.Namespace.ValueString(), &resp.Diagnostics)
200+
if !ok {
206201
return
207202
}
208203

209-
// Build API path from fields
210-
apiPath := BuildDistributeKeyPath(plan.Mount.ValueString(), plan.KMSName.ValueString(), plan.KeyName.ValueString())
204+
apiPath := plan.APIPath()
211205
writeData := map[string]interface{}{}
212206
hasChanges := false
213207

@@ -228,22 +222,28 @@ func (r *DistributeKeyResource) Update(ctx context.Context, req resource.UpdateR
228222
}
229223

230224
if hasChanges {
231-
writeResp, err := cli.Logical().WriteWithContext(ctx, apiPath, writeData)
232-
if err != nil {
225+
if _, err := cli.Logical().WriteWithContext(ctx, apiPath, writeData); err != nil {
233226
resp.Diagnostics.AddError(ErrUpdating(ResourceTypeKeyDistribution, apiPath, err))
234227
return
235228
}
229+
}
236230

237-
// Parse the write response to extract updated fields
238-
if writeResp != nil && writeResp.Data != nil {
239-
parseDistributeKeyResponse(ctx, writeResp.Data, &plan, &resp.Diagnostics)
240-
if resp.Diagnostics.HasError() {
241-
return
242-
}
243-
}
244-
} else {
245-
// No changes, preserve existing computed values
246-
plan.Versions = state.Versions
231+
// Read back the state from Vault
232+
responseData, exists := r.readDistributeKey(ctx, cli, apiPath, &resp.Diagnostics)
233+
if resp.Diagnostics.HasError() {
234+
return
235+
}
236+
if !exists {
237+
resp.Diagnostics.AddError(
238+
"Unexpected error after updating key distribution",
239+
fmt.Sprintf("Key distribution not found at path %q immediately after update", apiPath),
240+
)
241+
return
242+
}
243+
244+
plan.parseDistributeKeyResponse(ctx, responseData, &resp.Diagnostics)
245+
if resp.Diagnostics.HasError() {
246+
return
247247
}
248248

249249
resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...)
@@ -256,14 +256,12 @@ func (r *DistributeKeyResource) Delete(ctx context.Context, req resource.DeleteR
256256
return
257257
}
258258

259-
cli, err := client.GetClient(ctx, r.Meta(), data.Namespace.ValueString())
260-
if err != nil {
261-
resp.Diagnostics.AddError(errutil.ClientConfigureErr(err))
259+
cli, ok := r.getVaultClient(ctx, data.Namespace.ValueString(), &resp.Diagnostics)
260+
if !ok {
262261
return
263262
}
264263

265-
// Build API path from fields
266-
apiPath := BuildDistributeKeyPath(data.Mount.ValueString(), data.KMSName.ValueString(), data.KeyName.ValueString())
264+
apiPath := data.APIPath()
267265
if _, err := cli.Logical().DeleteWithContext(ctx, apiPath); err != nil {
268266
resp.Diagnostics.AddError(ErrDeleting(ResourceTypeKeyDistribution, apiPath, err))
269267
return
@@ -311,9 +309,36 @@ func (r *DistributeKeyResource) ImportState(ctx context.Context, req resource.Im
311309
}
312310
}
313311

314-
// parseDistributeKeyResponse extracts data from Vault API response data into the distribute key model
315-
// This works with both write responses and read responses from the key distribution API
316-
func parseDistributeKeyResponse(ctx context.Context, responseData map[string]interface{}, data *DistributeKeyResourceModel, diags *diag.Diagnostics) {
312+
// APIPath returns the Vault API path for this key distribution.
313+
func (m *DistributeKeyResourceModel) APIPath() string {
314+
return BuildDistributeKeyPath(m.Mount.ValueString(), m.KMSName.ValueString(), m.KeyName.ValueString())
315+
}
316+
317+
// getVaultClient returns a Vault client for the given namespace, adding a diagnostic on error.
318+
func (r *DistributeKeyResource) getVaultClient(ctx context.Context, namespace string, diags *diag.Diagnostics) (*vaultapi.Client, bool) {
319+
cli, err := client.GetClient(ctx, r.Meta(), namespace)
320+
if err != nil {
321+
diags.AddError(errutil.ClientConfigureErr(err))
322+
return nil, false
323+
}
324+
return cli, true
325+
}
326+
327+
// readDistributeKey reads the key distribution from Vault. Returns (data, true) if found, (nil, false) otherwise. API errors are added to diags.
328+
func (r *DistributeKeyResource) readDistributeKey(ctx context.Context, cli *vaultapi.Client, apiPath string, diags *diag.Diagnostics) (map[string]interface{}, bool) {
329+
vaultResp, err := cli.Logical().ReadWithContext(ctx, apiPath)
330+
if err != nil {
331+
diags.AddError(ErrReading(ResourceTypeKeyDistribution, apiPath, err))
332+
return nil, false
333+
}
334+
if vaultResp == nil {
335+
return nil, false
336+
}
337+
return vaultResp.Data, true
338+
}
339+
340+
// parseDistributeKeyResponse extracts data from Vault API response data into the distribute key model.
341+
func (data *DistributeKeyResourceModel) parseDistributeKeyResponse(ctx context.Context, responseData map[string]interface{}, diags *diag.Diagnostics) {
317342
if v, ok := responseData["purpose"].([]interface{}); ok {
318343
purposes, d := types.SetValueFrom(ctx, types.StringType, v)
319344
diags.Append(d...)

‎internal/vault/keymgmt/resource_keymgmt_replicate_key.go‎

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,15 @@ import (
99
"os"
1010
"strings"
1111

12+
"github.com/hashicorp/terraform-plugin-framework/diag"
1213
"github.com/hashicorp/terraform-plugin-framework/path"
1314
"github.com/hashicorp/terraform-plugin-framework/resource"
1415
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
1516
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
1617
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
1718
"github.com/hashicorp/terraform-plugin-framework/types"
1819
"github.com/hashicorp/terraform-plugin-log/tflog"
20+
vaultapi "github.com/hashicorp/vault/api"
1921

2022
"github.com/hashicorp/terraform-provider-vault/internal/consts"
2123
"github.com/hashicorp/terraform-provider-vault/internal/framework/base"
@@ -84,25 +86,20 @@ func (r *ReplicateKeyResource) Create(ctx context.Context, req resource.CreateRe
8486
return
8587
}
8688

87-
cli, err := client.GetClient(ctx, r.Meta(), data.Namespace.ValueString())
88-
if err != nil {
89-
resp.Diagnostics.AddError(errutil.ClientConfigureErr(err))
89+
cli, ok := r.getVaultClient(ctx, data.Namespace.ValueString(), &resp.Diagnostics)
90+
if !ok {
9091
return
9192
}
9293

93-
vaultPath := data.Mount.ValueString()
94-
kmsName := data.KMSName.ValueString()
95-
keyName := data.KeyName.ValueString()
96-
97-
kmsPath := BuildKMSPath(vaultPath, kmsName)
94+
kmsPath := data.KMSPath()
9895
kmsResp, err := cli.Logical().ReadWithContext(ctx, kmsPath)
9996
if err != nil {
10097
resp.Diagnostics.AddError(ErrReading(ResourceTypeKMSProvider, kmsPath, err))
10198
return
10299
}
103100

104101
if kmsResp == nil {
105-
resp.Diagnostics.AddError("KMS provider not found", fmt.Sprintf("KMS provider %s not found at %s", kmsName, kmsPath))
102+
resp.Diagnostics.AddError("KMS provider not found", fmt.Sprintf("KMS provider %s not found at %s", data.KMSName.ValueString(), kmsPath))
106103
return
107104
}
108105

@@ -116,9 +113,9 @@ func (r *ReplicateKeyResource) Create(ctx context.Context, req resource.CreateRe
116113
return
117114
}
118115

119-
apiPath := BuildReplicateKeyPath(vaultPath, kmsName, keyName)
120-
if _, err := cli.Logical().WriteWithContext(ctx, apiPath, map[string]interface{}{}); err != nil {
121-
resp.Diagnostics.AddError("Error replicating Key Management key", fmt.Sprintf("Error replicating key at %s: %s", apiPath, err))
116+
replicatePath := data.ReplicatePath()
117+
if _, err := cli.Logical().WriteWithContext(ctx, replicatePath, map[string]interface{}{}); err != nil {
118+
resp.Diagnostics.AddError("Error replicating Key Management key", fmt.Sprintf("Error replicating key at %s: %s", replicatePath, err))
122119
return
123120
}
124121

@@ -132,14 +129,13 @@ func (r *ReplicateKeyResource) Read(ctx context.Context, req resource.ReadReques
132129
return
133130
}
134131

135-
cli, err := client.GetClient(ctx, r.Meta(), data.Namespace.ValueString())
136-
if err != nil {
137-
resp.Diagnostics.AddError(errutil.ClientConfigureErr(err))
132+
cli, ok := r.getVaultClient(ctx, data.Namespace.ValueString(), &resp.Diagnostics)
133+
if !ok {
138134
return
139135
}
140136

141137
// Build base path to check if distribution exists
142-
basePath := BuildDistributeKeyPath(data.Mount.ValueString(), data.KMSName.ValueString(), data.KeyName.ValueString())
138+
basePath := data.DistributePath()
143139
vaultResp, err := cli.Logical().ReadWithContext(ctx, basePath)
144140
if err != nil {
145141
resp.Diagnostics.AddError(ErrReading(ResourceTypeKeyDistribution, basePath, err))
@@ -218,3 +214,28 @@ func (r *ReplicateKeyResource) ImportState(ctx context.Context, req resource.Imp
218214
)
219215
}
220216
}
217+
218+
// KMSPath returns the Vault API path for the KMS provider.
219+
func (m *ReplicateKeyResourceModel) KMSPath() string {
220+
return BuildKMSPath(m.Mount.ValueString(), m.KMSName.ValueString())
221+
}
222+
223+
// ReplicatePath returns the Vault API path for key replication.
224+
func (m *ReplicateKeyResourceModel) ReplicatePath() string {
225+
return BuildReplicateKeyPath(m.Mount.ValueString(), m.KMSName.ValueString(), m.KeyName.ValueString())
226+
}
227+
228+
// DistributePath returns the Vault API path for the key distribution (used to verify replication exists).
229+
func (m *ReplicateKeyResourceModel) DistributePath() string {
230+
return BuildDistributeKeyPath(m.Mount.ValueString(), m.KMSName.ValueString(), m.KeyName.ValueString())
231+
}
232+
233+
// getVaultClient returns a Vault client for the given namespace, adding a diagnostic on error.
234+
func (r *ReplicateKeyResource) getVaultClient(ctx context.Context, namespace string, diags *diag.Diagnostics) (*vaultapi.Client, bool) {
235+
cli, err := client.GetClient(ctx, r.Meta(), namespace)
236+
if err != nil {
237+
diags.AddError(errutil.ClientConfigureErr(err))
238+
return nil, false
239+
}
240+
return cli, true
241+
}

0 commit comments

Comments
 (0)