Skip to content

Commit 30bdd70

Browse files
committed
fix: Address PR review comments from knottnt
- Update ResolveReferencesForField function comment to reflect new generated output with ValidateCrossNamespaceReference call, three return values, and cross-namespace warning/condition logic - Remove stale --enable-cross-namespace-references flag from deployment.yaml.tpl (superseded by --enable-cross-namespace) - Remove stale enableCrossNamespaceReferences value from values.yaml.tpl (superseded by enableCrossNamespace)
1 parent 44b49f7 commit 30bdd70

3 files changed

Lines changed: 28 additions & 32 deletions

File tree

pkg/generate/code/resource_reference.go

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,11 @@ func ReferenceFieldsValidation(
122122
// are related to the given concrete field, determining whether its in a valid
123123
// condition and updating the concrete field with the referenced value.
124124
//
125+
// The generated code calls ackrt.ValidateCrossNamespaceReference to check
126+
// whether the reference targets a different namespace. If the cross-namespace
127+
// flag is disabled, the helper returns an error. If enabled and cross-namespace,
128+
// a warning is logged and a condition is set on the resource.
129+
//
125130
// Sample output (resolving a singular reference):
126131
//
127132
// if ko.Spec.APIRef != nil && ko.Spec.APIRef.From != nil {
@@ -130,6 +135,19 @@ func ReferenceFieldsValidation(
130135
// if arr.Name == nil || *arr.Name == "" {
131136
// return hasReferences, fmt.Errorf("provided resource reference is nil or empty: APIRef")
132137
// }
138+
// namespace, isCrossNs, err := ackrt.ValidateCrossNamespaceReference(
139+
// rm.cfg.EnableCrossNamespace,
140+
// ko.ObjectMeta.GetNamespace(),
141+
// arr.Namespace,
142+
// *arr.Name,
143+
// )
144+
// if err != nil {
145+
// return hasReferences, err
146+
// }
147+
// if isCrossNs {
148+
// ackrtlog.FromContext(ctx).Info("cross-namespace resource reference detected; ...")
149+
// setCrossNamespaceCondition(ko, crossNsMsg)
150+
// }
133151
// obj := &svcapitypes.API{}
134152
// if err := getReferencedResourceState_API(ctx, apiReader, obj, *arr.Name, namespace); err != nil {
135153
// return hasReferences, err
@@ -146,6 +164,16 @@ func ReferenceFieldsValidation(
146164
// if arr.Name == nil || *arr.Name == "" {
147165
// return hasReferences, fmt.Errorf("provided resource reference is nil or empty: SecurityGroupRefs")
148166
// }
167+
// namespace, isCrossNs, err := ackrt.ValidateCrossNamespaceReference(
168+
// rm.cfg.EnableCrossNamespace,
169+
// ko.ObjectMeta.GetNamespace(),
170+
// arr.Namespace,
171+
// *arr.Name,
172+
// )
173+
// if err != nil {
174+
// return hasReferences, err
175+
// }
176+
// if isCrossNs { ... }
149177
// obj := &ec2apitypes.SecurityGroup{}
150178
// if err := getReferencedResourceState_SecurityGroup(ctx, apiReader, obj, *arr.Name, namespace); err != nil {
151179
// return hasReferences, err
@@ -156,31 +184,6 @@ func ReferenceFieldsValidation(
156184
// ko.Spec.SecurityGroupIDs = append(ko.Spec.SecurityGroupIDs, (*string)(obj.Status.ID))
157185
// }
158186
// }
159-
//
160-
// Sample output (resolving nested lists of structs containing references):
161-
//
162-
// if ko.Spec.Notification != nil {
163-
// for f0idx, f0iter := range ko.Spec.Notification.LambdaFunctionConfigurations {
164-
// if f0iter.Filter != nil {
165-
// if f0iter.Filter.Key != nil {
166-
// for f1idx, f1iter := range f0iter.Filter.Key.FilterRules {
167-
// if f1iter.ValueRef != nil && f1iter.ValueRef.From != nil {
168-
// hasReferences = true
169-
// arr := f1iter.ValueRef.From
170-
// if arr.Name == nil || *arr.Name == "" {
171-
// return hasReferences, fmt.Errorf("provided resource reference is nil or empty: Notification.LambdaFunctionConfigurations.Filter.Key.FilterRules.ValueRef")
172-
// }
173-
// obj := &svcapitypes.Bucket{}
174-
// if err := getReferencedResourceState_Bucket(ctx, apiReader, obj, *arr.Name, namespace); err != nil {
175-
// return hasReferences, err
176-
// }
177-
// ko.Spec.Notification.LambdaFunctionConfigurations[f0idx].Filter.Key.FilterRules[f1idx].Value = (*string)(obj.Spec.Name)
178-
// }
179-
// }
180-
// }
181-
// }
182-
// }
183-
// }
184187
func ResolveReferencesForField(field *model.Field, sourceVarName string, indentLevel int) (string, error) {
185188
isListOfRefs := field.ShapeRef.Shape.Type == "list"
186189

templates/helm/templates/deployment.yaml.tpl

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@ spec:
105105
{{ "{{- end }}" }}
106106
- {{ "--enable-carm={{ .Values.enableCARM }}" }}
107107
- {{ "--enable-cross-namespace={{ .Values.enableCrossNamespace }}" }}
108-
- {{ "--enable-cross-namespace-references={{ .Values.enableCrossNamespaceReferences }}" }}
109108
image: {{ "{{ .Values.image.repository }}:{{ .Values.image.tag }}" }}
110109
imagePullPolicy: {{ "{{ .Values.image.pullPolicy }}" }}
111110
name: controller

templates/helm/values.yaml.tpl

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -185,12 +185,6 @@ enableCARM: true
185185
# that crosses namespace boundaries.
186186
enableCrossNamespace: true
187187

188-
# Enables cross-namespace resolution of AWSResourceReference entries.
189-
# When false (default), the controller rejects any reference whose
190-
# Namespace field differs from the namespace of the resource containing
191-
# the reference.
192-
enableCrossNamespaceReferences: false
193-
194188
# Configuration for feature gates. These are optional controller features that
195189
# can be individually enabled ("true") or disabled ("false") by adding key/value
196190
# pairs below.

0 commit comments

Comments
 (0)