Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix slice type sensitive fieldpath generation #355

Merged
merged 2 commits into from
Feb 26, 2024
Merged

Fix slice type sensitive fieldpath generation #355

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8bf78e8
Fix non-primitive type sensitive field generation
sergenyalcin Feb 23, 2024
b63f338
Change cases from string to type of FieldType
sergenyalcin Feb 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion pkg/types/field.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,13 @@ func NewSensitiveField(g *Builder, cfg *config.Resource, r *resource, sch *schem
return nil, true, nil
}
sfx := "SecretRef"
cfg.Sensitive.AddFieldPath(fieldPathWithWildcard(f.TerraformPaths), "spec.forProvider."+fieldPathWithWildcard(f.CRDPaths)+sfx)
switch f.FieldType.String() {
case "string", "*string", "map[string]string", "map[string]*string":
cfg.Sensitive.AddFieldPath(fieldPathWithWildcard(f.TerraformPaths), "spec.forProvider."+fieldPathWithWildcard(f.CRDPaths)+sfx)
case "[]string", "[]*string":
f.CRDPaths[len(f.CRDPaths)-2] = f.CRDPaths[len(f.CRDPaths)-2] + sfx
cfg.Sensitive.AddFieldPath(fieldPathWithWildcard(f.TerraformPaths), "spec.forProvider."+fieldPathWithWildcard(f.CRDPaths))
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we already have a validation enumeration under this switch block, how about getting rid of the enumeration here?

Suggested change
switch f.FieldType.String() {
case "string", "*string", "map[string]string", "map[string]*string":
cfg.Sensitive.AddFieldPath(fieldPathWithWildcard(f.TerraformPaths), "spec.forProvider."+fieldPathWithWildcard(f.CRDPaths)+sfx)
case "[]string", "[]*string":
f.CRDPaths[len(f.CRDPaths)-2] = f.CRDPaths[len(f.CRDPaths)-2] + sfx
cfg.Sensitive.AddFieldPath(fieldPathWithWildcard(f.TerraformPaths), "spec.forProvider."+fieldPathWithWildcard(f.CRDPaths))
}
switch f.FieldType.(type) {
case *types.Slice:
f.CRDPaths[len(f.CRDPaths)-2] = f.CRDPaths[len(f.CRDPaths)-2] + sfx
cfg.Sensitive.AddFieldPath(fieldPathWithWildcard(f.TerraformPaths), "spec.forProvider."+fieldPathWithWildcard(f.CRDPaths))
default:
cfg.Sensitive.AddFieldPath(fieldPathWithWildcard(f.TerraformPaths), "spec.forProvider."+fieldPathWithWildcard(f.CRDPaths)+sfx)
}

Also, do we need to handle the maps in a similar manner?

Copy link
Member Author

@sergenyalcin sergenyalcin Feb 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @ulucinar for that comment. It will be valuable to record an essential piece of information.

When I started to prepare this change, I first evaluated the map type together with the slice type and, put it in the same case, and tried generation. However, I got an index out of range error in this case. Then, in the debugging session I did, I saw that the fieldpath we generate for the fields of the map type does not contain the [*] expression at the end, similar to slices, but only the name of the field, like the string type. Therefore, I put the maps in the same case as the strings.

Calculated fieldpath examples

String: x.y.z
Map: x.y.z
Slice: x.y.z[*]

In short, we do not need to handle the map type here. It seems that the paved library can handle an expression without a wildcard correctly for maps but requires a wildcard for slices. At this point, we need a branch.

// todo(turkenh): do we need to support other field types as sensitive?
if f.FieldType.String() != "string" && f.FieldType.String() != "*string" && f.FieldType.String() != "[]string" &&
f.FieldType.String() != "[]*string" && f.FieldType.String() != "map[string]string" && f.FieldType.String() != "map[string]*string" {
Expand Down
Loading