Skip to content

Commit 350ffc5

Browse files
authored
Merge branch 'main' into tests/ipv6-only-environment
2 parents bdc569d + 826319f commit 350ffc5

File tree

12 files changed

+192
-35
lines changed

12 files changed

+192
-35
lines changed

.github/workflows/build.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ jobs:
182182
fail-build: false
183183

184184
- name: Upload scan result to GitHub Security tab
185-
uses: github/codeql-action/upload-sarif@2d92b76c45b91eb80fc44c74ce3fce0ee94e8f9d # v3.30.0
185+
uses: github/codeql-action/upload-sarif@f1f6e5f6af878fb37288ce1c627459e94dbf7d01 # v3.30.1
186186
if: ${{ !inputs.dry_run }}
187187
continue-on-error: true
188188
with:

.github/workflows/scorecards.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,6 @@ jobs:
6060

6161
# Upload the results to GitHub's code scanning dashboard.
6262
- name: "Upload to code-scanning"
63-
uses: github/codeql-action/upload-sarif@2d92b76c45b91eb80fc44c74ce3fce0ee94e8f9d # v3.30.0
63+
uses: github/codeql-action/upload-sarif@f1f6e5f6af878fb37288ce1c627459e94dbf7d01 # v3.30.1
6464
with:
6565
sarif_file: results.sarif

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ require (
1717
go.opentelemetry.io/otel v1.38.0
1818
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.38.0
1919
go.uber.org/zap v1.27.0
20-
golang.org/x/text v0.28.0
20+
golang.org/x/text v0.29.0
2121
google.golang.org/grpc v1.75.0
2222
google.golang.org/protobuf v1.36.8
2323
gopkg.in/evanphx/json-patch.v4 v4.13.0
@@ -77,7 +77,7 @@ require (
7777
golang.org/x/mod v0.27.0 // indirect
7878
golang.org/x/net v0.43.0 // indirect
7979
golang.org/x/oauth2 v0.30.0 // indirect
80-
golang.org/x/sync v0.16.0 // indirect
80+
golang.org/x/sync v0.17.0 // indirect
8181
golang.org/x/sys v0.35.0 // indirect
8282
golang.org/x/term v0.34.0 // indirect
8383
golang.org/x/time v0.9.0 // indirect

go.sum

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -270,8 +270,8 @@ golang.org/x/oauth2 v0.30.0/go.mod h1:B++QgG3ZKulg6sRPGD/mqlHQs5rB3Ml9erfeDY7xKl
270270
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
271271
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
272272
golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
273-
golang.org/x/sync v0.16.0 h1:ycBJEhp9p4vXvUZNszeOq0kGTPghopOL8q0fq3vstxw=
274-
golang.org/x/sync v0.16.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA=
273+
golang.org/x/sync v0.17.0 h1:l60nONMj9l5drqw6jlhIELNv9I0A4OFgRsG9k2oT9Ug=
274+
golang.org/x/sync v0.17.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI=
275275
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
276276
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
277277
golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
@@ -281,8 +281,8 @@ golang.org/x/term v0.34.0 h1:O/2T7POpk0ZZ7MAzMeWFSg6S5IpWd/RXDlM9hgM3DR4=
281281
golang.org/x/term v0.34.0/go.mod h1:5jC53AEywhIVebHgPVeg0mj8OD3VO9OzclacVrqpaAw=
282282
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
283283
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
284-
golang.org/x/text v0.28.0 h1:rhazDwis8INMIwQ4tpjLDzUhx6RlXqZNPEM0huQojng=
285-
golang.org/x/text v0.28.0/go.mod h1:U8nCwOR8jO/marOQ0QbDiOngZVEBB7MAiitBuMjXiNU=
284+
golang.org/x/text v0.29.0 h1:1neNs90w9YzJ9BocxfsQNHKuAT4pkghyXc4nhZ6sJvk=
285+
golang.org/x/text v0.29.0/go.mod h1:7MhJOA9CD2qZyOKYazxdYMF85OwPdEr9jTtBpO7ydH4=
286286
golang.org/x/time v0.9.0 h1:EsRrnYcQiGH+5FfbgvV4AP7qEZstoyrHB0DzarOQ4ZY=
287287
golang.org/x/time v0.9.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM=
288288
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=

internal/controller/state/conditions/conditions.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,18 @@ const (
147147
// when a policy cannot be applied due to the ancestor limit being reached.
148148
PolicyMessageAncestorLimitReached = "Policies cannot be applied because the ancestor status list " +
149149
"has reached the maximum size. The following policies have been ignored:"
150+
151+
// BackendTLSPolicyReasonInvalidCACertificateRef is used with the "ResolvedRefs" condition when a
152+
// CACertificateRef refers to a resource that cannot be resolved or is misconfigured.
153+
BackendTLSPolicyReasonInvalidCACertificateRef v1alpha2.PolicyConditionReason = "InvalidCACertificateRef"
154+
155+
// BackendTLSPolicyReasonInvalidKind is used with the "ResolvedRefs" condition when a
156+
// CACertificateRef refers to an unknown or unsupported kind of resource.
157+
BackendTLSPolicyReasonInvalidKind v1alpha2.PolicyConditionReason = "InvalidKind"
158+
159+
// BackendTLSPolicyReasonNoValidCACertificate is used with the "Accepted" condition when all
160+
// CACertificateRefs are invalid.
161+
BackendTLSPolicyReasonNoValidCACertificate v1alpha2.PolicyConditionReason = "NoValidCACertificate"
150162
)
151163

152164
// Condition defines a condition to be reported in the status of resources.
@@ -1053,3 +1065,47 @@ func NewClientSettingsPolicyAffected() Condition {
10531065
Message: "ClientSettingsPolicy is applied to the resource",
10541066
}
10551067
}
1068+
1069+
// NewBackendTLSPolicyResolvedRefs returns a Condition that indicates that all CACertificateRefs
1070+
// in the BackendTLSPolicy are resolved.
1071+
func NewBackendTLSPolicyResolvedRefs() Condition {
1072+
return Condition{
1073+
Type: string(GatewayResolvedRefs),
1074+
Status: metav1.ConditionTrue,
1075+
Reason: string(GatewayResolvedRefs),
1076+
Message: "All CACertificateRefs are resolved",
1077+
}
1078+
}
1079+
1080+
// NewBackendTLSPolicyInvalidCACertificateRef returns a Condition that indicates that a
1081+
// CACertificateRef in the BackendTLSPolicy refers to a resource that cannot be resolved or is misconfigured.
1082+
func NewBackendTLSPolicyInvalidCACertificateRef(message string) Condition {
1083+
return Condition{
1084+
Type: string(GatewayResolvedRefs),
1085+
Status: metav1.ConditionFalse,
1086+
Reason: string(BackendTLSPolicyReasonInvalidCACertificateRef),
1087+
Message: message,
1088+
}
1089+
}
1090+
1091+
// NewBackendTLSPolicyInvalidKind returns a Condition that indicates that a CACertificateRef
1092+
// in the BackendTLSPolicy refers to an unknown or unsupported kind of resource.
1093+
func NewBackendTLSPolicyInvalidKind(message string) Condition {
1094+
return Condition{
1095+
Type: string(GatewayResolvedRefs),
1096+
Status: metav1.ConditionFalse,
1097+
Reason: string(BackendTLSPolicyReasonInvalidKind),
1098+
Message: message,
1099+
}
1100+
}
1101+
1102+
// NewBackendTLSPolicyNoValidCACertificate returns a Condition that indicates that all
1103+
// CACertificateRefs in the BackendTLSPolicy are invalid.
1104+
func NewBackendTLSPolicyNoValidCACertificate(message string) Condition {
1105+
return Condition{
1106+
Type: string(v1alpha2.PolicyConditionAccepted),
1107+
Status: metav1.ConditionFalse,
1108+
Reason: string(BackendTLSPolicyReasonNoValidCACertificate),
1109+
Message: message,
1110+
}
1111+
}

internal/controller/state/graph/backend_refs.go

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@ func createBackendRef(
216216
ref.Namespace,
217217
string(ref.Name),
218218
route.Source.GetNamespace(),
219+
svcPort,
219220
)
220221
if err != nil {
221222
backendRef := BackendRef{
@@ -307,30 +308,56 @@ func findBackendTLSPolicyForService(
307308
refNamespace *gatewayv1.Namespace,
308309
refName,
309310
routeNamespace string,
311+
servicePort v1.ServicePort,
310312
) (*BackendTLSPolicy, error) {
311313
var beTLSPolicy *BackendTLSPolicy
314+
var conflictingPolicies []*BackendTLSPolicy
312315
var err error
313316

314317
refNs := routeNamespace
315318
if refNamespace != nil {
316319
refNs = string(*refNamespace)
317320
}
318321

322+
// First pass: find all policies targeting this service and port
319323
for _, btp := range backendTLSPolicies {
320324
btpNs := btp.Source.Namespace
321325
for _, targetRef := range btp.Source.Spec.TargetRefs {
322326
if string(targetRef.Name) == refName && btpNs == refNs {
323-
if beTLSPolicy != nil {
327+
// Check if this policy applies to the specific port we're interested in
328+
if targetRef.SectionName != nil {
329+
// Policy targets a specific port by name
330+
if servicePort.Name != string(*targetRef.SectionName) {
331+
// This policy targets a different port, skip it
332+
continue
333+
}
334+
}
335+
// Policy applies to all ports (no sectionName) or matches our port
336+
337+
if beTLSPolicy == nil {
338+
beTLSPolicy = btp
339+
} else {
340+
// Found a conflict - determine which policy wins
324341
if sort.LessClientObject(btp.Source, beTLSPolicy.Source) {
342+
// btp wins, beTLSPolicy loses
343+
conflictingPolicies = append(conflictingPolicies, beTLSPolicy)
325344
beTLSPolicy = btp
345+
} else {
346+
// beTLSPolicy wins, btp loses
347+
conflictingPolicies = append(conflictingPolicies, btp)
326348
}
327-
} else {
328-
beTLSPolicy = btp
329349
}
330350
}
331351
}
332352
}
333353

354+
// Set conflicted conditions on losing policies
355+
for _, conflictedPolicy := range conflictingPolicies {
356+
conflictedPolicy.IsReferenced = true
357+
conflictedPolicy.Conditions = append(conflictedPolicy.Conditions,
358+
conditions.NewPolicyConflicted("Conflicts with another BackendTLSPolicy targeting the same Service"))
359+
}
360+
334361
if beTLSPolicy != nil {
335362
beTLSPolicy.IsReferenced = true
336363
if !beTLSPolicy.Valid {

internal/controller/state/graph/backend_refs_test.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1707,7 +1707,13 @@ func TestFindBackendTLSPolicyForService(t *testing.T) {
17071707
t.Parallel()
17081708
g := NewWithT(t)
17091709

1710-
btp, err := findBackendTLSPolicyForService(test.backendTLSPolicies, ref.Namespace, string(ref.Name), "test")
1710+
btp, err := findBackendTLSPolicyForService(
1711+
test.backendTLSPolicies,
1712+
ref.Namespace,
1713+
string(ref.Name),
1714+
"test",
1715+
v1.ServicePort{Name: "https", Port: 443},
1716+
)
17111717

17121718
g.Expect(btp.Source.Name).To(Equal(test.expectedBtpName))
17131719
g.Expect(err).ToNot(HaveOccurred())

internal/controller/state/graph/backend_tls_policy.go

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -81,17 +81,22 @@ func validateBackendTLSPolicy(
8181

8282
caCertRefs := backendTLSPolicy.Spec.Validation.CACertificateRefs
8383
wellKnownCerts := backendTLSPolicy.Spec.Validation.WellKnownCACertificates
84+
85+
// Check mutual exclusivity
8486
switch {
8587
case len(caCertRefs) > 0 && wellKnownCerts != nil:
8688
valid = false
8789
msg := "CACertificateRefs and WellKnownCACertificates are mutually exclusive"
8890
conds = append(conds, conditions.NewPolicyInvalid(msg))
8991

9092
case len(caCertRefs) > 0:
91-
if err := validateBackendTLSCACertRef(backendTLSPolicy, configMapResolver, secretResolver); err != nil {
93+
certConds := validateBackendTLSCACertRef(backendTLSPolicy, configMapResolver, secretResolver)
94+
if len(certConds) > 0 {
9295
valid = false
93-
conds = append(conds, conditions.NewPolicyInvalid(
94-
fmt.Sprintf("invalid CACertificateRef: %s", err.Error())))
96+
conds = append(conds, certConds...)
97+
} else if valid {
98+
// Only set ResolvedRefs to true if CACertificateRefs are valid AND overall policy is valid
99+
conds = append(conds, conditions.NewBackendTLSPolicyResolvedRefs())
95100
}
96101

97102
case wellKnownCerts != nil:
@@ -103,8 +108,12 @@ func validateBackendTLSPolicy(
103108

104109
default:
105110
valid = false
106-
conds = append(conds, conditions.NewPolicyInvalid("CACertRefs and WellKnownCACerts are both nil"))
111+
conds = append(
112+
conds,
113+
conditions.NewPolicyInvalid("either CACertificateRefs or WellKnownCACertificates must be specified"),
114+
)
107115
}
116+
108117
return valid, ignored, conds
109118
}
110119

@@ -123,11 +132,11 @@ func validateBackendTLSCACertRef(
123132
btp *v1alpha3.BackendTLSPolicy,
124133
configMapResolver *configMapResolver,
125134
secretResolver *secretResolver,
126-
) error {
135+
) []conditions.Condition {
127136
if len(btp.Spec.Validation.CACertificateRefs) != 1 {
128137
path := field.NewPath("validation.caCertificateRefs")
129138
valErr := field.TooMany(path, len(btp.Spec.Validation.CACertificateRefs), 1)
130-
return valErr
139+
return []conditions.Condition{conditions.NewPolicyInvalid(valErr.Error())}
131140
}
132141

133142
selectedCertRef := btp.Spec.Validation.CACertificateRefs[0]
@@ -136,13 +145,19 @@ func validateBackendTLSCACertRef(
136145
if !slices.Contains(allowedCaCertKinds, selectedCertRef.Kind) {
137146
path := field.NewPath("validation.caCertificateRefs[0].kind")
138147
valErr := field.NotSupported(path, btp.Spec.Validation.CACertificateRefs[0].Kind, allowedCaCertKinds)
139-
return valErr
148+
return []conditions.Condition{
149+
conditions.NewBackendTLSPolicyInvalidKind(valErr.Error()),
150+
conditions.NewBackendTLSPolicyNoValidCACertificate("No valid CACertificateRef found"),
151+
}
140152
}
141153
if selectedCertRef.Group != "" &&
142154
selectedCertRef.Group != "core" {
143155
path := field.NewPath("validation.caCertificateRefs[0].group")
144156
valErr := field.NotSupported(path, selectedCertRef.Group, []string{"", "core"})
145-
return valErr
157+
return []conditions.Condition{
158+
conditions.NewBackendTLSPolicyInvalidKind(valErr.Error()),
159+
conditions.NewBackendTLSPolicyNoValidCACertificate("No valid CACertificateRef found"),
160+
}
146161
}
147162
nsName := types.NamespacedName{
148163
Namespace: btp.Namespace,
@@ -153,15 +168,21 @@ func validateBackendTLSCACertRef(
153168
case "ConfigMap":
154169
if err := configMapResolver.resolve(nsName); err != nil {
155170
path := field.NewPath("validation.caCertificateRefs[0]")
156-
return field.Invalid(path, selectedCertRef, err.Error())
171+
valErr := field.Invalid(path, selectedCertRef, err.Error())
172+
return []conditions.Condition{
173+
conditions.NewBackendTLSPolicyInvalidCACertificateRef(valErr.Error()),
174+
conditions.NewBackendTLSPolicyNoValidCACertificate("No valid CACertificateRef found"),
175+
}
157176
}
158177
case "Secret":
159178
if err := secretResolver.resolve(nsName); err != nil {
160179
path := field.NewPath("validation.caCertificateRefs[0]")
161-
return field.Invalid(path, selectedCertRef, err.Error())
180+
valErr := field.Invalid(path, selectedCertRef, err.Error())
181+
return []conditions.Condition{
182+
conditions.NewBackendTLSPolicyInvalidCACertificateRef(valErr.Error()),
183+
conditions.NewBackendTLSPolicyNoValidCACertificate("No valid CACertificateRef found"),
184+
}
162185
}
163-
default:
164-
return fmt.Errorf("invalid certificate reference kind %q", selectedCertRef.Kind)
165186
}
166187
return nil
167188
}

internal/controller/state/graph/backend_tls_policy_test.go

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -479,13 +479,59 @@ func TestValidateBackendTLSPolicy(t *testing.T) {
479479

480480
valid, ignored, conds := validateBackendTLSPolicy(test.tlsPolicy, configMapResolver, secretMapResolver)
481481

482-
if !test.isValid && !test.ignored {
483-
g.Expect(conds).To(HaveLen(1))
484-
} else {
485-
g.Expect(conds).To(BeEmpty())
486-
}
487482
g.Expect(valid).To(Equal(test.isValid))
488483
g.Expect(ignored).To(Equal(test.ignored))
484+
485+
if test.isValid && !test.ignored {
486+
// Valid cases should have ResolvedRefs condition set to true
487+
if len(test.tlsPolicy.Spec.Validation.CACertificateRefs) > 0 {
488+
g.Expect(conds).To(HaveLen(1))
489+
g.Expect(conds[0].Type).To(Equal(string(conditions.GatewayResolvedRefs)))
490+
g.Expect(conds[0].Status).To(Equal(metav1.ConditionTrue))
491+
} else {
492+
// WellKnownCACertificates case - no specific condition expected
493+
g.Expect(conds).To(BeEmpty())
494+
}
495+
} else if !test.isValid && !test.ignored {
496+
// Invalid cases should have at least one condition
497+
g.Expect(conds).ToNot(BeEmpty())
498+
499+
// Check specific condition types based on the test case
500+
switch test.name {
501+
case "invalid ca cert ref kind":
502+
// Should have InvalidKind condition and NoValidCACertificate condition
503+
g.Expect(conds).To(HaveLen(2))
504+
g.Expect(conds[0].Type).To(Equal(string(conditions.GatewayResolvedRefs)))
505+
g.Expect(conds[0].Status).To(Equal(metav1.ConditionFalse))
506+
g.Expect(conds[0].Reason).To(Equal(string(conditions.BackendTLSPolicyReasonInvalidKind)))
507+
g.Expect(conds[1].Type).To(Equal(string(v1alpha2.PolicyConditionAccepted)))
508+
g.Expect(conds[1].Status).To(Equal(metav1.ConditionFalse))
509+
g.Expect(conds[1].Reason).To(Equal(string(conditions.BackendTLSPolicyReasonNoValidCACertificate)))
510+
case "invalid ca cert ref name":
511+
// Should have InvalidCACertificateRef condition and NoValidCACertificate condition
512+
g.Expect(conds).To(HaveLen(2))
513+
g.Expect(conds[0].Type).To(Equal(string(conditions.GatewayResolvedRefs)))
514+
g.Expect(conds[0].Status).To(Equal(metav1.ConditionFalse))
515+
g.Expect(conds[0].Reason).To(Equal(string(conditions.BackendTLSPolicyReasonInvalidCACertificateRef)))
516+
g.Expect(conds[1].Type).To(Equal(string(v1alpha2.PolicyConditionAccepted)))
517+
g.Expect(conds[1].Status).To(Equal(metav1.ConditionFalse))
518+
g.Expect(conds[1].Reason).To(Equal(string(conditions.BackendTLSPolicyReasonNoValidCACertificate)))
519+
case "invalid ca cert ref group":
520+
// Should have InvalidKind condition and NoValidCACertificate condition
521+
g.Expect(conds).To(HaveLen(2))
522+
g.Expect(conds[0].Type).To(Equal(string(conditions.GatewayResolvedRefs)))
523+
g.Expect(conds[0].Status).To(Equal(metav1.ConditionFalse))
524+
g.Expect(conds[0].Reason).To(Equal(string(conditions.BackendTLSPolicyReasonInvalidKind)))
525+
g.Expect(conds[1].Type).To(Equal(string(v1alpha2.PolicyConditionAccepted)))
526+
g.Expect(conds[1].Status).To(Equal(metav1.ConditionFalse))
527+
g.Expect(conds[1].Reason).To(Equal(string(conditions.BackendTLSPolicyReasonNoValidCACertificate)))
528+
default:
529+
// Other invalid cases should have generic PolicyInvalid condition
530+
g.Expect(conds).To(HaveLen(1))
531+
g.Expect(conds[0].Type).To(Equal(string(v1alpha2.PolicyConditionAccepted)))
532+
g.Expect(conds[0].Status).To(Equal(metav1.ConditionFalse))
533+
}
534+
}
489535
})
490536
}
491537
}

internal/controller/state/graph/graph_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ func TestBuildGraph(t *testing.T) {
5050
}
5151

5252
btpAcceptedConds := []conditions.Condition{
53+
conditions.NewBackendTLSPolicyResolvedRefs(),
5354
conditions.NewPolicyAccepted(),
5455
conditions.NewPolicyAccepted(),
5556
conditions.NewPolicyAccepted(),

0 commit comments

Comments
 (0)