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.

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

Handle sensitive input maps via SecretReference #138

Merged
merged 3 commits into from
Nov 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
73 changes: 30 additions & 43 deletions pkg/resource/sensitive.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,40 +206,41 @@ func GetSensitiveParameters(ctx context.Context, client SecretClient, from runti

switch k := v.(type) {
case map[string]any:
if hasMapValue(k) {
sel := &map[string]v1.SecretKeySelector{}
if err = pavedJSON.GetValueInto(expandedJSONPath, sel); err != nil {
_, ok := k["key"]
if !ok {
// This is a special case where we have a "SecretReference" without a selected "key". This happens
// when there is an input field of type map[string]string (or map[string]*string).
// In this case, we need to get the entire secret data and fill it in the terraform state as a map.
// This is the only case where we have one-to-many mapping between json and tf paths.
ref := &v1.SecretReference{}
if err = pavedJSON.GetValueInto(expandedJSONPath, ref); err != nil {
return errors.Wrapf(err, errFmtCannotGetSecretKeySelectorAsMap, expandedJSONPath)
}
sensitives := make(map[string]any)
for key, value := range *sel {
sensitive, err = client.GetSecretValue(ctx, value)
if resource.IgnoreNotFound(err) != nil {
return errors.Wrapf(err, errFmtCannotGetSecretValue, sel)
}

// If referenced k8s secret is deleted before the MR, we pass empty string for the sensitive
// field to be able to destroy the resource.
if kerrors.IsNotFound(err) {
sensitive = []byte("")
}
sensitives[key] = string(sensitive)
}
if err := setSensitiveParametersWithPaved(pavedTF, expandedJSONPath, tfPath, mapping, sensitives); err != nil {
return err
}
} else {
sel := &v1.SecretKeySelector{}
if err = pavedJSON.GetValueInto(expandedJSONPath, sel); err != nil {
return errors.Wrapf(err, errFmtCannotGetSecretKeySelector, expandedJSONPath)
}
sensitive, err = client.GetSecretValue(ctx, *sel)
data, err := client.GetSecretData(ctx, ref)
// We don't want to fail if the secret is not found. Otherwise, we won't be able to delete the
// resource if secret is deleted before. This is quite expected when both secret and resource
// got deleted in parallel.
if resource.IgnoreNotFound(err) != nil {
return errors.Wrapf(err, errFmtCannotGetSecretValue, sel)
return errors.Wrapf(err, errFmtCannotGetSecretValue, ref)
}
if err := setSensitiveParametersWithPaved(pavedTF, expandedJSONPath, tfPath, mapping, string(sensitive)); err != nil {
return err
for key, value := range data {
if err = pavedTF.SetValue(fmt.Sprintf("%s.%s", tfPath, key), string(value)); err != nil {
return errors.Wrapf(err, "cannot set string as terraform attribute for fieldpath %q", fmt.Sprintf("%s.%s", tfPath, key))
}
}
continue
}

sel := &v1.SecretKeySelector{}
if err = pavedJSON.GetValueInto(expandedJSONPath, sel); err != nil {
return errors.Wrapf(err, errFmtCannotGetSecretKeySelector, expandedJSONPath)
}
sensitive, err = client.GetSecretValue(ctx, *sel)
if resource.IgnoreNotFound(err) != nil {
return errors.Wrapf(err, errFmtCannotGetSecretValue, sel)
}
if err := setSensitiveParametersWithPaved(pavedTF, expandedJSONPath, tfPath, mapping, string(sensitive)); err != nil {
return err
}
case []any:
sel := &[]v1.SecretKeySelector{}
Expand Down Expand Up @@ -418,17 +419,3 @@ func setSensitiveAttributesToValuesMap(e, i any, k, fp string, vals map[string][
vals[fmt.Sprintf("%s%s.%v", prefixAttribute, k, i)] = []byte(value)
return nil
}

// This for loop accesses the first value of selectorMap to determine the map's value type.
func hasMapValue(selectorMap map[string]any) bool {
for _, v := range selectorMap {
switch v.(type) {
case map[string]any:
return true
case string:
return false
}
}

return false
}
40 changes: 11 additions & 29 deletions pkg/resource/sensitive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -610,39 +610,21 @@ func TestGetSensitiveParameters(t *testing.T) {
},
},
},
"SingleNoWildcardWithMap": {
"SingleNoWildcardWithSecretReference": {
args: args{
clientFn: func(client *mocks.MockSecretClient) {
client.EXPECT().GetSecretValue(gomock.Any(), gomock.Eq(xpv1.SecretKeySelector{
SecretReference: xpv1.SecretReference{
Name: "db-passwords",
Namespace: "crossplane-system",
},
Key: "admin",
})).Return([]byte("admin_pwd"), nil)
client.EXPECT().GetSecretValue(gomock.Any(), gomock.Eq(xpv1.SecretKeySelector{
SecretReference: xpv1.SecretReference{
Name: "db-passwords",
Namespace: "crossplane-system",
},
Key: "system",
})).Return([]byte("system_pwd"), nil)
client.EXPECT().GetSecretData(gomock.Any(), gomock.Eq(&xpv1.SecretReference{
Name: "db-passwords",
Namespace: "crossplane-system",
})).Return(map[string][]byte{"admin": []byte("admin_pwd"), "system": []byte("system_pwd")}, nil)
},
from: &unstructured.Unstructured{
Object: map[string]any{
"spec": map[string]any{
"forProvider": map[string]any{
"passwordsSecretRef": map[string]any{
"pwd1": map[string]any{
"key": "admin",
"name": "db-passwords",
"namespace": "crossplane-system",
},
"pwd2": map[string]any{
"key": "system",
"name": "db-passwords",
"namespace": "crossplane-system",
},
"dbPasswordsSecretRef": map[string]any{
"name": "db-passwords",
"namespace": "crossplane-system",
},
},
},
Expand All @@ -652,15 +634,15 @@ func TestGetSensitiveParameters(t *testing.T) {
"some_other_key": "some_other_value",
},
mapping: map[string]string{
"db_passwords": "spec.forProvider.passwordsSecretRef",
"db_passwords": "spec.forProvider.dbPasswordsSecretRef",
},
},
want: want{
out: map[string]any{
"some_other_key": "some_other_value",
"db_passwords": map[string]any{
"pwd1": "admin_pwd",
"pwd2": "system_pwd",
"admin": "admin_pwd",
"system": "system_pwd",
},
},
},
Expand Down
9 changes: 7 additions & 2 deletions pkg/types/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,13 @@ func (g *Builder) buildSchema(f *Field, cfg *config.Resource, names []string, r
return types.NewPointer(types.Universe.Lookup("string").Type()), nil
case schema.TypeMap, schema.TypeList, schema.TypeSet:
names = append(names, f.Name.Camel)
f.TerraformPaths = append(f.TerraformPaths, wildcard)
f.CRDPaths = append(f.CRDPaths, wildcard)
if f.Schema.Type != schema.TypeMap {
// We don't want to have a many-to-many relationship in case of a Map, since we use SecretReference as
// the type of XP field. In this case, we want to have a one-to-many relationship which is handled at
// runtime in the controller.
f.TerraformPaths = append(f.TerraformPaths, wildcard)
f.CRDPaths = append(f.CRDPaths, wildcard)
}
var elemType types.Type
switch et := f.Schema.Elem.(type) {
case schema.ValueType:
Expand Down
2 changes: 1 addition & 1 deletion pkg/types/field.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func NewSensitiveField(g *Builder, cfg *config.Resource, r *resource, sch *schem
case "[]string", "[]*string":
f.FieldType = types.NewSlice(typeSecretKeySelector)
case "map[string]string", "map[string]*string":
f.FieldType = types.NewMap(types.Universe.Lookup("string").Type(), typeSecretKeySelector)
f.FieldType = typeSecretReference
}
f.TransformedName = name.NewFromCamel(f.FieldNameCamel).LowerCamelComputed
f.JSONTag = f.TransformedName
Expand Down
5 changes: 5 additions & 0 deletions pkg/types/reference.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ var (
types.NewStruct(nil, nil),
nil,
)
typeSecretReference types.Type = types.NewNamed(
types.NewTypeName(token.NoPos, types.NewPackage(PackagePathXPCommonAPIs, "v1"), "SecretReference", nil),
types.NewStruct(nil, nil),
nil,
)
commentOptional = &comments.Comment{
Options: markers.Options{
KubebuilderOptions: markers.KubebuilderOptions{
Expand Down