Skip to content

Commit f0dc0d8

Browse files
committed
fix: Move secret cross-namespace validation into runtime
Drop the per-call secret validation block from generated sdk.go. The generated code only wrapped SecretValueFromReference call sites in sdk.go, missing custom update functions (e.g. RDS db_cluster) and hooks that call SecretValueFromReference directly. Validation now lives entirely in the runtime's SecretValueFromReference (aws-controllers-k8s/runtime), so every caller is covered without generated boilerplate. - set_sdk.go: generated secret code reverts to a plain SecretValueFromReference call; no ResolveCrossNamespaceReferenceString - sdk.go.tpl: drop the now-unused ackrt import and its suppression entry - set_sdk_test.go: remove the validation block from 7 expected outputs Addresses review feedback from knottnt on #699.
1 parent eab25a9 commit f0dc0d8

3 files changed

Lines changed: 10 additions & 136 deletions

File tree

pkg/generate/code/set_sdk.go

Lines changed: 10 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,25 +1099,15 @@ func setSDKForContainer(
10991099

11001100
// setSDKForSecret returns a string of Go code that sets a target variable to
11011101
// the value of a Secret when the type of the source variable is a
1102-
// SecretKeyReference. It first calls ResolveCrossNamespaceReferenceString to
1103-
// validate cross-namespace access (and emit a deprecation warning + condition
1104-
// when the reference is cross-namespace), then fetches the secret value.
1102+
// SecretKeyReference.
1103+
//
1104+
// Cross-namespace validation (and the Phase 1 deprecation warning) is
1105+
// performed inside the runtime's SecretValueFromReference, so it is not
1106+
// emitted here. This ensures every caller is covered, including custom
1107+
// update functions and hooks that call SecretValueFromReference directly.
11051108
//
11061109
// The Go code output from this function looks like this:
11071110
//
1108-
// secretNamespace, err := ackrt.ResolveCrossNamespaceReferenceString(
1109-
// ctx,
1110-
// rm.cfg.EnableCrossNamespace,
1111-
// &r.ko.Status.Conditions,
1112-
// ackrt.CrossNamespaceRefKindSecret,
1113-
// r.ko.ObjectMeta.GetNamespace(),
1114-
// ko.Spec.MasterUserPassword.Namespace,
1115-
// ko.Spec.MasterUserPassword.Name,
1116-
// )
1117-
// if err != nil {
1118-
// return nil, err
1119-
// }
1120-
// ko.Spec.MasterUserPassword.Namespace = secretNamespace
11211111
// tmpSecret, err := rm.rr.SecretValueFromReference(ctx, ko.Spec.MasterUserPassword)
11221112
// if err != nil {
11231113
// return nil, ackrequeue.Needed(err)
@@ -1145,33 +1135,10 @@ func setSDKForSecret(
11451135
indent := strings.Repeat("\t", indentLevel)
11461136
secVar := "tmpSecret"
11471137

1148-
// Resolve cross-namespace access before fetching the secret
1149-
// secretNamespace, err := ackrt.ResolveCrossNamespaceReferenceString(
1150-
// ctx,
1151-
// rm.cfg.EnableCrossNamespace,
1152-
// &r.ko.Status.Conditions,
1153-
// ackrt.CrossNamespaceRefKindSecret,
1154-
// r.ko.ObjectMeta.GetNamespace(),
1155-
// sourceVarName.Namespace,
1156-
// sourceVarName.Name,
1157-
// )
1158-
out += fmt.Sprintf(
1159-
"%s\tsecretNamespace, err := ackrt.ResolveCrossNamespaceReferenceString(\n",
1160-
indent,
1161-
)
1162-
out += fmt.Sprintf("%s\t\tctx,\n", indent)
1163-
out += fmt.Sprintf("%s\t\trm.cfg.EnableCrossNamespace,\n", indent)
1164-
out += fmt.Sprintf("%s\t\t&r.ko.Status.Conditions,\n", indent)
1165-
out += fmt.Sprintf("%s\t\tackrt.CrossNamespaceRefKindSecret,\n", indent)
1166-
out += fmt.Sprintf("%s\t\tr.ko.ObjectMeta.GetNamespace(),\n", indent)
1167-
out += fmt.Sprintf("%s\t\t%s.Namespace,\n", indent, sourceVarName)
1168-
out += fmt.Sprintf("%s\t\t%s.Name,\n", indent, sourceVarName)
1169-
out += fmt.Sprintf("%s\t)\n", indent)
1170-
out += fmt.Sprintf("%s\tif err != nil {\n", indent)
1171-
out += fmt.Sprintf("%s\t\treturn nil, err\n", indent)
1172-
out += fmt.Sprintf("%s\t}\n", indent)
1173-
// Override the secret reference namespace with the resolved namespace
1174-
out += fmt.Sprintf("%s\t%s.Namespace = secretNamespace\n", indent, sourceVarName)
1138+
// Cross-namespace validation for the secret reference is performed inside
1139+
// the runtime's SecretValueFromReference, so that every call site is
1140+
// covered (including custom update functions and hooks). No per-call
1141+
// validation is generated here.
11751142

11761143
// tmpSecret, err := rm.rr.SecretValueFromReference(ctx, ko.Spec.MasterUserPassword)
11771144
out += fmt.Sprintf(

pkg/generate/code/set_sdk_test.go

Lines changed: 0 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -105,19 +105,6 @@ func TestSetSDK_MemoryDB_User_Create(t *testing.T) {
105105
for _, f1f0iter := range r.ko.Spec.AuthenticationMode.Passwords {
106106
var f1f0elem string
107107
if f1f0iter != nil {
108-
secretNamespace, err := ackrt.ResolveCrossNamespaceReferenceString(
109-
ctx,
110-
rm.cfg.EnableCrossNamespace,
111-
&r.ko.Status.Conditions,
112-
ackrt.CrossNamespaceRefKindSecret,
113-
r.ko.ObjectMeta.GetNamespace(),
114-
f1f0iter.Namespace,
115-
f1f0iter.Name,
116-
)
117-
if err != nil {
118-
return nil, err
119-
}
120-
f1f0iter.Namespace = secretNamespace
121108
tmpSecret, err := rm.rr.SecretValueFromReference(ctx, f1f0iter)
122109
if err != nil {
123110
return nil, ackrequeue.Needed(err)
@@ -221,19 +208,6 @@ func TestSetSDK_OpenSearch_Domain_Create(t *testing.T) {
221208
f3f4.MasterUserName = r.ko.Spec.AdvancedSecurityOptions.MasterUserOptions.MasterUserName
222209
}
223210
if r.ko.Spec.AdvancedSecurityOptions.MasterUserOptions.MasterUserPassword != nil {
224-
secretNamespace, err := ackrt.ResolveCrossNamespaceReferenceString(
225-
ctx,
226-
rm.cfg.EnableCrossNamespace,
227-
&r.ko.Status.Conditions,
228-
ackrt.CrossNamespaceRefKindSecret,
229-
r.ko.ObjectMeta.GetNamespace(),
230-
r.ko.Spec.AdvancedSecurityOptions.MasterUserOptions.MasterUserPassword.Namespace,
231-
r.ko.Spec.AdvancedSecurityOptions.MasterUserOptions.MasterUserPassword.Name,
232-
)
233-
if err != nil {
234-
return nil, err
235-
}
236-
r.ko.Spec.AdvancedSecurityOptions.MasterUserOptions.MasterUserPassword.Namespace = secretNamespace
237211
tmpSecret, err := rm.rr.SecretValueFromReference(ctx, r.ko.Spec.AdvancedSecurityOptions.MasterUserOptions.MasterUserPassword)
238212
if err != nil {
239213
return nil, ackrequeue.Needed(err)
@@ -1515,19 +1489,6 @@ func TestSetSDK_RDS_DBInstance_Create(t *testing.T) {
15151489
res.ManageMasterUserPassword = r.ko.Spec.ManageMasterUserPassword
15161490
}
15171491
if r.ko.Spec.MasterUserPassword != nil {
1518-
secretNamespace, err := ackrt.ResolveCrossNamespaceReferenceString(
1519-
ctx,
1520-
rm.cfg.EnableCrossNamespace,
1521-
&r.ko.Status.Conditions,
1522-
ackrt.CrossNamespaceRefKindSecret,
1523-
r.ko.ObjectMeta.GetNamespace(),
1524-
r.ko.Spec.MasterUserPassword.Namespace,
1525-
r.ko.Spec.MasterUserPassword.Name,
1526-
)
1527-
if err != nil {
1528-
return nil, err
1529-
}
1530-
r.ko.Spec.MasterUserPassword.Namespace = secretNamespace
15311492
tmpSecret, err := rm.rr.SecretValueFromReference(ctx, r.ko.Spec.MasterUserPassword)
15321493
if err != nil {
15331494
return nil, ackrequeue.Needed(err)
@@ -1818,19 +1779,6 @@ func TestSetSDK_RDS_DBInstance_Update(t *testing.T) {
18181779
}
18191780
if delta.DifferentAt("Spec.MasterUserPassword") {
18201781
if r.ko.Spec.MasterUserPassword != nil {
1821-
secretNamespace, err := ackrt.ResolveCrossNamespaceReferenceString(
1822-
ctx,
1823-
rm.cfg.EnableCrossNamespace,
1824-
&r.ko.Status.Conditions,
1825-
ackrt.CrossNamespaceRefKindSecret,
1826-
r.ko.ObjectMeta.GetNamespace(),
1827-
r.ko.Spec.MasterUserPassword.Namespace,
1828-
r.ko.Spec.MasterUserPassword.Name,
1829-
)
1830-
if err != nil {
1831-
return nil, err
1832-
}
1833-
r.ko.Spec.MasterUserPassword.Namespace = secretNamespace
18341782
tmpSecret, err := rm.rr.SecretValueFromReference(ctx, r.ko.Spec.MasterUserPassword)
18351783
if err != nil {
18361784
return nil, ackrequeue.Needed(err)
@@ -2370,19 +2318,6 @@ func TestSetSDK_MQ_Broker_Create(t *testing.T) {
23702318
f18elem.Groups = aws.ToStringSlice(f18iter.Groups)
23712319
}
23722320
if f18iter.Password != nil {
2373-
secretNamespace, err := ackrt.ResolveCrossNamespaceReferenceString(
2374-
ctx,
2375-
rm.cfg.EnableCrossNamespace,
2376-
&r.ko.Status.Conditions,
2377-
ackrt.CrossNamespaceRefKindSecret,
2378-
r.ko.ObjectMeta.GetNamespace(),
2379-
f18iter.Password.Namespace,
2380-
f18iter.Password.Name,
2381-
)
2382-
if err != nil {
2383-
return nil, err
2384-
}
2385-
f18iter.Password.Namespace = secretNamespace
23862321
tmpSecret, err := rm.rr.SecretValueFromReference(ctx, f18iter.Password)
23872322
if err != nil {
23882323
return nil, ackrequeue.Needed(err)
@@ -4734,19 +4669,6 @@ func TestSetSDK_Lambda_Function_EnvironmentVariable_MapOfSecrets_Create(t *testi
47344669
for f4f0key, f4f0valiter := range r.ko.Spec.Environment.Variables {
47354670
var f4f0val string
47364671
if f4f0valiter != nil {
4737-
secretNamespace, err := ackrt.ResolveCrossNamespaceReferenceString(
4738-
ctx,
4739-
rm.cfg.EnableCrossNamespace,
4740-
&r.ko.Status.Conditions,
4741-
ackrt.CrossNamespaceRefKindSecret,
4742-
r.ko.ObjectMeta.GetNamespace(),
4743-
f4f0valiter.Namespace,
4744-
f4f0valiter.Name,
4745-
)
4746-
if err != nil {
4747-
return nil, err
4748-
}
4749-
f4f0valiter.Namespace = secretNamespace
47504672
tmpSecret, err := rm.rr.SecretValueFromReference(ctx, f4f0valiter)
47514673
if err != nil {
47524674
return nil, ackrequeue.Needed(err)
@@ -4884,19 +4806,6 @@ func TestSetSDK_Lambda_Function_EnvironmentVariable_MapOfSecrets_Update(t *testi
48844806
for f2f0key, f2f0valiter := range r.ko.Spec.Environment.Variables {
48854807
var f2f0val string
48864808
if f2f0valiter != nil {
4887-
secretNamespace, err := ackrt.ResolveCrossNamespaceReferenceString(
4888-
ctx,
4889-
rm.cfg.EnableCrossNamespace,
4890-
&r.ko.Status.Conditions,
4891-
ackrt.CrossNamespaceRefKindSecret,
4892-
r.ko.ObjectMeta.GetNamespace(),
4893-
f2f0valiter.Namespace,
4894-
f2f0valiter.Name,
4895-
)
4896-
if err != nil {
4897-
return nil, err
4898-
}
4899-
f2f0valiter.Namespace = secretNamespace
49004809
tmpSecret, err := rm.rr.SecretValueFromReference(ctx, f2f0valiter)
49014810
if err != nil {
49024811
return nil, ackrequeue.Needed(err)

templates/pkg/resource/sdk.go.tpl

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare"
1616
ackerr "github.com/aws-controllers-k8s/runtime/pkg/errors"
1717
ackrequeue "github.com/aws-controllers-k8s/runtime/pkg/requeue"
18-
ackrt "github.com/aws-controllers-k8s/runtime/pkg/runtime"
1918
ackrtlog "github.com/aws-controllers-k8s/runtime/pkg/runtime/log"
2019
"github.com/aws/aws-sdk-go-v2/aws"
2120
svcsdk "github.com/aws/aws-sdk-go-v2/service/{{ .ServicePackageName }}"
@@ -40,7 +39,6 @@ var (
4039
_ = fmt.Sprintf("")
4140
_ = &ackrequeue.NoRequeue{}
4241
_ = &aws.Config{}
43-
_ = ackrt.ValidateCrossNamespaceReferenceString
4442
)
4543

4644
// sdkFind returns SDK-specific information about a supplied resource

0 commit comments

Comments
 (0)