Skip to content

Commit f19ae99

Browse files
benashzbenashz
committed
Remove read operation on entity alias update
The identityEntityAliasUpdate function was performing a read operation when it should not have been. The read operation should be constrained to the defined ReadContext. - add new GetAPIRequestData() function that generalizes the translation of schema.ResourceData values to their Vault request data equivalents.
1 parent a15cad6 commit f19ae99

4 files changed

Lines changed: 191 additions & 54 deletions

File tree

‎util/util.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,3 +322,25 @@ func CheckMountEnabled(client *api.Client, path string) (bool, error) {
322322

323323
return ok, nil
324324
}
325+
326+
// GetAPIRequestData to pass to Vault from schema.ResourceData.
327+
// The fieldMap specifies the schema field to its vault constituent.
328+
// If the vault field is empty, then two fields are mapped 1:1.
329+
func GetAPIRequestData(d *schema.ResourceData, fieldMap map[string]string) map[string]interface{} {
330+
data := make(map[string]interface{})
331+
for k1, k2 := range fieldMap {
332+
if k2 == "" {
333+
k2 = k1
334+
}
335+
336+
sv := d.Get(k1)
337+
switch v := sv.(type) {
338+
case *schema.Set:
339+
data[k2] = v.List()
340+
default:
341+
data[k2] = sv
342+
}
343+
}
344+
345+
return data
346+
}

‎util/util_test.go‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,3 +255,113 @@ func TestPathParameters(t *testing.T) {
255255
})
256256
}
257257
}
258+
259+
func TestGetAPIRequestData(t *testing.T) {
260+
tests := []struct {
261+
name string
262+
d map[string]*schema.Schema
263+
m map[string]string
264+
sm map[string]interface{}
265+
want map[string]interface{}
266+
}{
267+
{
268+
name: "basic-default",
269+
d: map[string]*schema.Schema{
270+
"name": {
271+
Type: schema.TypeString,
272+
},
273+
},
274+
m: map[string]string{
275+
"name": "",
276+
},
277+
sm: map[string]interface{}{
278+
"name": "bob",
279+
},
280+
want: map[string]interface{}{
281+
"name": "bob",
282+
},
283+
},
284+
{
285+
name: "basic-remap",
286+
d: map[string]*schema.Schema{
287+
"name": {
288+
Type: schema.TypeString,
289+
},
290+
},
291+
m: map[string]string{
292+
"name": "nom",
293+
},
294+
sm: map[string]interface{}{
295+
"name": "bob",
296+
},
297+
want: map[string]interface{}{
298+
"nom": "bob",
299+
},
300+
},
301+
{
302+
name: "map",
303+
d: map[string]*schema.Schema{
304+
"name": {
305+
Type: schema.TypeString,
306+
},
307+
"parts": {
308+
Type: schema.TypeMap,
309+
},
310+
},
311+
m: map[string]string{
312+
"name": "",
313+
"parts": "",
314+
},
315+
sm: map[string]interface{}{
316+
"name": "bob",
317+
"parts": map[string]interface{}{
318+
"bolt": "0.60",
319+
},
320+
},
321+
want: map[string]interface{}{
322+
"name": "bob",
323+
"parts": map[string]interface{}{
324+
"bolt": "0.60",
325+
},
326+
},
327+
},
328+
{
329+
name: "set",
330+
d: map[string]*schema.Schema{
331+
"name": {
332+
Type: schema.TypeString,
333+
},
334+
"parts": {
335+
Type: schema.TypeSet,
336+
Elem: &schema.Schema{
337+
Type: schema.TypeString,
338+
},
339+
},
340+
},
341+
m: map[string]string{
342+
"name": "",
343+
"parts": "",
344+
},
345+
sm: map[string]interface{}{
346+
"name": "alice",
347+
"parts": []interface{}{
348+
"bolt",
349+
},
350+
},
351+
want: map[string]interface{}{
352+
"name": "alice",
353+
"parts": []interface{}{
354+
"bolt",
355+
},
356+
},
357+
},
358+
}
359+
for _, tt := range tests {
360+
t.Run(tt.name, func(t *testing.T) {
361+
r := schema.TestResourceDataRaw(t, tt.d, tt.sm)
362+
if got := GetAPIRequestData(r, tt.m); !reflect.DeepEqual(got, tt.want) {
363+
t.Errorf("GetAPIRequestData() = %v, want %v", got, tt.want)
364+
}
365+
})
366+
}
367+
}

‎vault/resource_identity_entity_alias.go‎

Lines changed: 28 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/hashicorp/vault/api"
1212

1313
"github.com/hashicorp/terraform-provider-vault/internal/identity/entity"
14+
"github.com/hashicorp/terraform-provider-vault/util"
1415
)
1516

1617
func identityEntityAliasResource() *schema.Resource {
@@ -54,29 +55,26 @@ func identityEntityAliasResource() *schema.Resource {
5455
}
5556

5657
func identityEntityAliasCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
57-
lock, unlock := getEntityAliasLockFuncs(d)
58+
lock, unlock := getEntityLockFuncs(d, entity.RootAliasIDPath)
5859
lock()
5960
defer unlock()
6061

6162
client := meta.(*api.Client)
6263

6364
path := entity.RootAliasPath
6465
name := d.Get("name").(string)
65-
mountAccessor := d.Get("mount_accessor").(string)
66-
canonicalID := d.Get("canonical_id").(string)
67-
customMetadata := d.Get("custom_metadata").(map[string]interface{})
68-
69-
data := map[string]interface{}{
70-
"name": name,
71-
"mount_accessor": mountAccessor,
72-
"canonical_id": canonicalID,
73-
"custom_metadata": customMetadata,
74-
}
66+
data := util.GetAPIRequestData(d, map[string]string{
67+
"name": "",
68+
"mount_accessor": "",
69+
"canonical_id": "",
70+
"custom_metadata": "",
71+
})
7572

7673
diags := diag.Diagnostics{}
7774

7875
var duplicates []string
7976

77+
mountAccessor := data["mount_accessor"].(string)
8078
aliases, err := entity.FindAliases(client, &entity.FindAliasParams{
8179
Name: name,
8280
MountAccessor: mountAccessor,
@@ -139,7 +137,7 @@ func identityEntityAliasCreate(ctx context.Context, d *schema.ResourceData, meta
139137
}
140138

141139
func identityEntityAliasUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
142-
lock, unlock := getEntityAliasLockFuncs(d)
140+
lock, unlock := getEntityLockFuncs(d, entity.RootAliasIDPath)
143141
lock()
144142
defer unlock()
145143

@@ -151,44 +149,21 @@ func identityEntityAliasUpdate(ctx context.Context, d *schema.ResourceData, meta
151149

152150
diags := diag.Diagnostics{}
153151

154-
resp, err := client.Logical().Read(path)
155-
if err != nil {
156-
diags = append(diags, diag.Diagnostic{
157-
Severity: diag.Error,
158-
Summary: fmt.Sprintf("error reading entity alias %q: %s", id, err),
159-
})
160-
161-
return diags
162-
}
163-
164-
data := map[string]interface{}{
165-
"name": resp.Data["name"],
166-
"mount_accessor": resp.Data["mount_accessor"],
167-
"canonical_id": resp.Data["canonical_id"],
168-
}
169-
170-
if name, ok := d.GetOk("name"); ok {
171-
data["name"] = name
172-
}
173-
if mountAccessor, ok := d.GetOk("mount_accessor"); ok {
174-
data["mount_accessor"] = mountAccessor
175-
}
176-
if canonicalID, ok := d.GetOk("canonical_id"); ok {
177-
data["canonical_id"] = canonicalID
178-
}
179-
180-
data["custom_metadata"] = d.Get("custom_metadata").(map[string]interface{})
181-
182-
_, err = client.Logical().Write(path, data)
183-
184-
if err != nil {
152+
data := util.GetAPIRequestData(d, map[string]string{
153+
"name": "",
154+
"mount_accessor": "",
155+
"canonical_id": "",
156+
"custom_metadata": "",
157+
})
158+
if _, err := client.Logical().Write(path, data); err != nil {
185159
diags = append(diags, diag.Diagnostic{
186160
Severity: diag.Error,
187161
Summary: fmt.Sprintf("error updating entity alias %q: %s", id, err),
188162
})
189163

190164
return diags
191165
}
166+
192167
log.Printf("[DEBUG] Updated entity alias %q", id)
193168

194169
return identityEntityAliasRead(ctx, d, meta)
@@ -203,22 +178,21 @@ func identityEntityAliasRead(ctx context.Context, d *schema.ResourceData, meta i
203178
diags := diag.Diagnostics{}
204179

205180
log.Printf("[DEBUG] Reading entity alias %q from %q", id, path)
206-
resp, err := client.Logical().Read(path)
181+
resp, err := readEntity(client, path, d.IsNewResource())
207182
if err != nil {
183+
if isIdentityNotFoundError(err) {
184+
log.Printf("[WARN] entity alias %q not found, removing from state", id)
185+
d.SetId("")
186+
return diags
187+
}
188+
208189
diags = append(diags, diag.Diagnostic{
209190
Severity: diag.Error,
210191
Summary: fmt.Sprintf("error reading entity alias %q: %s", id, err),
211192
})
212193

213194
return diags
214195
}
215-
log.Printf("[DEBUG] Read entity alias %s", id)
216-
if resp == nil {
217-
log.Printf("[WARN] entity alias %q not found, removing from state", id)
218-
d.SetId("")
219-
220-
return diags
221-
}
222196

223197
d.SetId(resp.Data["id"].(string))
224198
for _, k := range []string{"name", "mount_accessor", "canonical_id", "custom_metadata"} {
@@ -236,7 +210,7 @@ func identityEntityAliasRead(ctx context.Context, d *schema.ResourceData, meta i
236210
}
237211

238212
func identityEntityAliasDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
239-
lock, unlock := getEntityAliasLockFuncs(d)
213+
lock, unlock := getEntityLockFuncs(d, entity.RootAliasIDPath)
240214
lock()
241215
defer unlock()
242216

@@ -262,9 +236,9 @@ func identityEntityAliasDelete(ctx context.Context, d *schema.ResourceData, meta
262236
return diags
263237
}
264238

265-
func getEntityAliasLockFuncs(d *schema.ResourceData) (func(), func()) {
239+
func getEntityLockFuncs(d *schema.ResourceData, root string) (func(), func()) {
266240
mountAccessor := d.Get("mount_accessor").(string)
267-
lockKey := strings.Join([]string{entity.RootAliasIDPath, mountAccessor}, "/")
241+
lockKey := strings.Join([]string{root, mountAccessor}, "/")
268242
lock := func() {
269243
vaultMutexKV.Lock(lockKey)
270244
}

‎vault/resource_identity_entity_alias_test.go‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,37 @@ resource "vault_identity_entity_alias" "test2" {
183183
ImportState: true,
184184
ImportStateVerify: true,
185185
},
186+
{
187+
// delete one of the alias's to ensure an update operation re-creates it.
188+
PreConfig: func() {
189+
client := testProvider.Meta().(*api.Client)
190+
aliases, err := entity.FindAliases(client, &entity.FindAliasParams{
191+
Name: alias,
192+
})
193+
if err != nil {
194+
t.Fatal(err)
195+
}
196+
197+
if len(aliases) != 1 {
198+
t.Fatalf("expected Alias %q not found in Vault", alias)
199+
}
200+
201+
_, err = client.Logical().Delete(entity.JoinAliasID(aliases[0].ID))
202+
if err != nil {
203+
t.Fatal(err)
204+
}
205+
},
206+
Config: fmt.Sprintf(configTmpl, alias, alias+"-2"),
207+
Check: resource.ComposeTestCheckFunc(
208+
resource.TestCheckResourceAttrPair(
209+
aliasResource1, "mount_accessor",
210+
aliasResource2, "mount_accessor"),
211+
resource.TestCheckResourceAttr(
212+
aliasResource1, "name", alias),
213+
resource.TestCheckResourceAttr(
214+
aliasResource2, "name", alias+"-2"),
215+
),
216+
},
186217
{
187218
// duplicate during an update operation
188219
// this should result Vault catching the duplicate and returning an error.

0 commit comments

Comments
 (0)