Skip to content

Commit 2443919

Browse files
sjbermansjberman
committed
Update validation struct name and add condition
1 parent fd592da commit 2443919

File tree

13 files changed

+90
-61
lines changed

13 files changed

+90
-61
lines changed

‎internal/mode/static/policies/clientsettings/validator.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func NewValidator(genericValidator validation.GenericValidator) *Validator {
2828
}
2929

3030
// Validate validates the spec of a ClientSettingsPolicy.
31-
func (v *Validator) Validate(policy policies.Policy, _ *policies.GlobalPolicySettings) []conditions.Condition {
31+
func (v *Validator) Validate(policy policies.Policy, _ *policies.ValidationContext) []conditions.Condition {
3232
csp := helpers.MustCastObject[*ngfAPI.ClientSettingsPolicy](policy)
3333

3434
if err := validateTargetRef(csp.Spec.TargetRef); err != nil {

‎internal/mode/static/policies/manager.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ type GenerateFunc func(policy Policy, globalSettings *ngfAPI.NginxProxy) []byte
2020
//counterfeiter:generate . Validator
2121
type Validator interface {
2222
// Validate validates an NGF Policy.
23-
Validate(policy Policy, globalSettings *GlobalPolicySettings) []conditions.Condition
23+
Validate(policy Policy, policyValidationCtx *ValidationContext) []conditions.Condition
2424
// Conflicts returns true if the two Policies conflict.
2525
Conflicts(a, b Policy) bool
2626
}
@@ -75,15 +75,15 @@ func (m *Manager) Generate(policy Policy, globalSettings *ngfAPI.NginxProxy) []b
7575
}
7676

7777
// Validate validates the policy.
78-
func (m *Manager) Validate(policy Policy, globalSettings *GlobalPolicySettings) []conditions.Condition {
78+
func (m *Manager) Validate(policy Policy, policyValidationCtx *ValidationContext) []conditions.Condition {
7979
gvk := m.mustExtractGVK(policy)
8080

8181
validator, ok := m.validators[gvk]
8282
if !ok {
8383
panic(fmt.Sprintf("no validator registered for policy %T", policy))
8484
}
8585

86-
return validator.Validate(policy, globalSettings)
86+
return validator.Validate(policy, policyValidationCtx)
8787
}
8888

8989
// Conflicts returns true if the policies conflict.

‎internal/mode/static/policies/manager_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ var _ = Describe("Policy Manager", func() {
4343
mustExtractGVK,
4444
policies.ManagerConfig{
4545
Validator: &policiesfakes.FakeValidator{
46-
ValidateStub: func(_ policies.Policy, _ *policies.GlobalPolicySettings) []conditions.Condition {
46+
ValidateStub: func(_ policies.Policy, _ *policies.ValidationContext) []conditions.Condition {
4747
return []conditions.Condition{staticConds.NewPolicyInvalid("apple error")}
4848
},
4949
ConflictsStub: func(_ policies.Policy, _ policies.Policy) bool { return true },
@@ -55,7 +55,7 @@ var _ = Describe("Policy Manager", func() {
5555
},
5656
policies.ManagerConfig{
5757
Validator: &policiesfakes.FakeValidator{
58-
ValidateStub: func(_ policies.Policy, _ *policies.GlobalPolicySettings) []conditions.Condition {
58+
ValidateStub: func(_ policies.Policy, _ *policies.ValidationContext) []conditions.Condition {
5959
return []conditions.Condition{staticConds.NewPolicyInvalid("orange error")}
6060
},
6161
ConflictsStub: func(_ policies.Policy, _ policies.Policy) bool { return false },

‎internal/mode/static/policies/observability/validator.go

+11-3
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,23 @@ func NewValidator(genericValidator validation.GenericValidator) *Validator {
3030
// Validate validates the spec of an ObservabilityPolicy.
3131
func (v *Validator) Validate(
3232
policy policies.Policy,
33-
globalSettings *policies.GlobalPolicySettings,
33+
policyValidationCtx *policies.ValidationContext,
3434
) []conditions.Condition {
3535
obs, ok := policy.(*ngfAPI.ObservabilityPolicy)
3636
if !ok {
3737
panic(fmt.Sprintf("expected ObservabilityPolicy, got: %T", policy))
3838
}
3939

40-
if globalSettings == nil || !globalSettings.NginxProxyValid {
41-
return []conditions.Condition{staticConds.NewPolicyNotAcceptedNginxProxyNotSet()}
40+
if policyValidationCtx == nil || !policyValidationCtx.NginxProxyValid {
41+
return []conditions.Condition{
42+
staticConds.NewPolicyNotAcceptedNginxProxyNotSet(staticConds.PolicyMessageNginxProxyInvalid),
43+
}
44+
}
45+
46+
if !policyValidationCtx.TelemetryEnabled {
47+
return []conditions.Condition{
48+
staticConds.NewPolicyNotAcceptedNginxProxyNotSet(staticConds.PolicyMessageTelemetryNotEnabled),
49+
}
4250
}
4351

4452
if err := validateTargetRefs(obs.Spec.TargetRefs); err != nil {

‎internal/mode/static/policies/observability/validator_test.go

+39-28
Original file line numberDiff line numberDiff line change
@@ -52,91 +52,102 @@ func createModifiedPolicy(mod policyModFunc) *ngfAPI.ObservabilityPolicy {
5252
}
5353

5454
func TestValidator_Validate(t *testing.T) {
55+
validCtx := &policies.ValidationContext{
56+
NginxProxyValid: true,
57+
TelemetryEnabled: true,
58+
}
59+
5560
tests := []struct {
56-
name string
57-
policy *ngfAPI.ObservabilityPolicy
58-
globalSettings *policies.GlobalPolicySettings
59-
expErrSubstrings []string
61+
name string
62+
policy *ngfAPI.ObservabilityPolicy
63+
policyValidationCtx *policies.ValidationContext
64+
expErrSubstrings []string
6065
}{
6166
{
62-
name: "global settings are nil",
67+
name: "validation context is nil",
6368
policy: createValidPolicy(),
6469
expErrSubstrings: []string{"NginxProxy configuration is either invalid or not attached"},
6570
},
6671
{
67-
name: "global settings are invalid",
68-
policy: createValidPolicy(),
69-
globalSettings: &policies.GlobalPolicySettings{NginxProxyValid: false},
70-
expErrSubstrings: []string{"NginxProxy configuration is either invalid or not attached"},
72+
name: "validation context is invalid",
73+
policy: createValidPolicy(),
74+
policyValidationCtx: &policies.ValidationContext{NginxProxyValid: false},
75+
expErrSubstrings: []string{"NginxProxy configuration is either invalid or not attached"},
76+
},
77+
{
78+
name: "telemetry is not enabled",
79+
policy: createValidPolicy(),
80+
policyValidationCtx: &policies.ValidationContext{NginxProxyValid: true, TelemetryEnabled: false},
81+
expErrSubstrings: []string{"Telemetry is not enabled"},
7182
},
7283
{
7384
name: "invalid target ref; unsupported group",
7485
policy: createModifiedPolicy(func(p *ngfAPI.ObservabilityPolicy) *ngfAPI.ObservabilityPolicy {
7586
p.Spec.TargetRefs[0].Group = "Unsupported"
7687
return p
7788
}),
78-
globalSettings: &policies.GlobalPolicySettings{NginxProxyValid: true},
79-
expErrSubstrings: []string{"spec.targetRefs.group"},
89+
policyValidationCtx: validCtx,
90+
expErrSubstrings: []string{"spec.targetRefs.group"},
8091
},
8192
{
8293
name: "invalid target ref; unsupported kind",
8394
policy: createModifiedPolicy(func(p *ngfAPI.ObservabilityPolicy) *ngfAPI.ObservabilityPolicy {
8495
p.Spec.TargetRefs[0].Kind = "Unsupported"
8596
return p
8697
}),
87-
globalSettings: &policies.GlobalPolicySettings{NginxProxyValid: true},
88-
expErrSubstrings: []string{"spec.targetRefs.kind"},
98+
policyValidationCtx: validCtx,
99+
expErrSubstrings: []string{"spec.targetRefs.kind"},
89100
},
90101
{
91102
name: "invalid strategy",
92103
policy: createModifiedPolicy(func(p *ngfAPI.ObservabilityPolicy) *ngfAPI.ObservabilityPolicy {
93104
p.Spec.Tracing.Strategy = "invalid"
94105
return p
95106
}),
96-
globalSettings: &policies.GlobalPolicySettings{NginxProxyValid: true},
97-
expErrSubstrings: []string{"spec.tracing.strategy"},
107+
policyValidationCtx: validCtx,
108+
expErrSubstrings: []string{"spec.tracing.strategy"},
98109
},
99110
{
100111
name: "invalid context",
101112
policy: createModifiedPolicy(func(p *ngfAPI.ObservabilityPolicy) *ngfAPI.ObservabilityPolicy {
102113
p.Spec.Tracing.Context = helpers.GetPointer[ngfAPI.TraceContext]("invalid")
103114
return p
104115
}),
105-
globalSettings: &policies.GlobalPolicySettings{NginxProxyValid: true},
106-
expErrSubstrings: []string{"spec.tracing.context"},
116+
policyValidationCtx: validCtx,
117+
expErrSubstrings: []string{"spec.tracing.context"},
107118
},
108119
{
109120
name: "invalid span name",
110121
policy: createModifiedPolicy(func(p *ngfAPI.ObservabilityPolicy) *ngfAPI.ObservabilityPolicy {
111122
p.Spec.Tracing.SpanName = helpers.GetPointer("invalid$$$")
112123
return p
113124
}),
114-
globalSettings: &policies.GlobalPolicySettings{NginxProxyValid: true},
115-
expErrSubstrings: []string{"spec.tracing.spanName"},
125+
policyValidationCtx: validCtx,
126+
expErrSubstrings: []string{"spec.tracing.spanName"},
116127
},
117128
{
118129
name: "invalid span attribute key",
119130
policy: createModifiedPolicy(func(p *ngfAPI.ObservabilityPolicy) *ngfAPI.ObservabilityPolicy {
120131
p.Spec.Tracing.SpanAttributes[0].Key = "invalid$$$"
121132
return p
122133
}),
123-
globalSettings: &policies.GlobalPolicySettings{NginxProxyValid: true},
124-
expErrSubstrings: []string{"spec.tracing.spanAttributes.key"},
134+
policyValidationCtx: validCtx,
135+
expErrSubstrings: []string{"spec.tracing.spanAttributes.key"},
125136
},
126137
{
127138
name: "invalid span attribute value",
128139
policy: createModifiedPolicy(func(p *ngfAPI.ObservabilityPolicy) *ngfAPI.ObservabilityPolicy {
129140
p.Spec.Tracing.SpanAttributes[0].Value = "invalid$$$"
130141
return p
131142
}),
132-
globalSettings: &policies.GlobalPolicySettings{NginxProxyValid: true},
133-
expErrSubstrings: []string{"spec.tracing.spanAttributes.value"},
143+
policyValidationCtx: validCtx,
144+
expErrSubstrings: []string{"spec.tracing.spanAttributes.value"},
134145
},
135146
{
136-
name: "valid",
137-
policy: createValidPolicy(),
138-
globalSettings: &policies.GlobalPolicySettings{NginxProxyValid: true},
139-
expErrSubstrings: nil,
147+
name: "valid",
148+
policy: createValidPolicy(),
149+
policyValidationCtx: validCtx,
150+
expErrSubstrings: nil,
140151
},
141152
}
142153

@@ -146,7 +157,7 @@ func TestValidator_Validate(t *testing.T) {
146157
t.Run(test.name, func(t *testing.T) {
147158
g := NewWithT(t)
148159

149-
conds := v.Validate(test.policy, test.globalSettings)
160+
conds := v.Validate(test.policy, test.policyValidationCtx)
150161

151162
if len(test.expErrSubstrings) == 0 {
152163
g.Expect(conds).To(BeEmpty())

‎internal/mode/static/policies/policiesfakes/fake_validator.go

+6-6
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎internal/mode/static/policies/policy.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,11 @@ type ConfigGenerator interface {
2626
Generate(policy Policy, globalSettings *ngfAPI.NginxProxy) []byte
2727
}
2828

29-
// GlobalPolicySettings are settings from the current state of the graph that may be
29+
// ValidationContext contains context from the current state of the graph that may be
3030
// needed for policy validation if certain policies rely on those global settings.
31-
type GlobalPolicySettings struct {
32-
NginxProxyValid bool
31+
type ValidationContext struct {
32+
NginxProxyValid bool
33+
TelemetryEnabled bool
3334
}
3435

3536
// We generate a mock of ObjectKind so that we can create fake policies and set their GVKs.

‎internal/mode/static/state/conditions/conditions.go

+10-2
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,14 @@ const (
7777
// PolicyReasonNginxProxyConfigNotSet is used with the "PolicyAccepted" condition when the
7878
// NginxProxy resource is missing or invalid.
7979
PolicyReasonNginxProxyConfigNotSet v1alpha2.PolicyConditionReason = "NginxProxyConfigNotSet"
80+
81+
// PolicyMessageNginxProxyInvalid is a message used with the PolicyReasonNginxProxyConfigNotSet reason
82+
// when the NginxProxy resource is either invalid or not attached.
83+
PolicyMessageNginxProxyInvalid = "The NginxProxy configuration is either invalid or not attached to the GatewayClass"
84+
85+
// PolicyMessageTelemetryNotEnabled is a message used with the PolicyReasonNginxProxyConfigNotSet reason
86+
// when telemetry is not enabled in the NginxProxy resource.
87+
PolicyMessageTelemetryNotEnabled = "Telemetry is not enabled in the NgixnProxy resource"
8088
)
8189

8290
// NewTODO returns a Condition that can be used as a placeholder for a condition that is not yet implemented.
@@ -646,11 +654,11 @@ func NewPolicyTargetNotFound(msg string) conditions.Condition {
646654

647655
// NewPolicyNotAcceptedNginxProxyNotSet returns a Condition that indicates that the Policy is not accepted
648656
// because it relies in the NginxProxy configuration which is missing or invalid.
649-
func NewPolicyNotAcceptedNginxProxyNotSet() conditions.Condition {
657+
func NewPolicyNotAcceptedNginxProxyNotSet(msg string) conditions.Condition {
650658
return conditions.Condition{
651659
Type: string(v1alpha2.PolicyConditionAccepted),
652660
Status: metav1.ConditionFalse,
653661
Reason: string(PolicyReasonNginxProxyConfigNotSet),
654-
Message: "The NginxProxy configuration is either invalid or not attached to the GatewayClass",
662+
Message: msg,
655663
}
656664
}

‎internal/mode/static/state/graph/graph.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ func BuildGraph(
175175
validators validation.Validators,
176176
protectedPorts ProtectedPorts,
177177
) *Graph {
178-
policySettings := &policies.GlobalPolicySettings{}
178+
policyValidationCtx := &policies.ValidationContext{}
179179

180180
processedGwClasses, gcExists := processGatewayClasses(state.GatewayClasses, gcName, controllerName)
181181
if gcExists && processedGwClasses.Winner == nil {
@@ -188,7 +188,8 @@ func BuildGraph(
188188
if gc != nil && npCfg != nil {
189189
for _, cond := range gc.Conditions {
190190
if cond.Type == string(conditions.GatewayClassResolvedRefs) && cond.Status == metav1.ConditionTrue {
191-
policySettings.NginxProxyValid = true
191+
policyValidationCtx.NginxProxyValid = true
192+
policyValidationCtx.TelemetryEnabled = npCfg.Spec.Telemetry != nil && npCfg.Spec.Telemetry.Exporter != nil
192193
break
193194
}
194195
}
@@ -231,7 +232,7 @@ func BuildGraph(
231232
validators.PolicyValidator,
232233
processedGws,
233234
routes,
234-
policySettings,
235+
policyValidationCtx,
235236
)
236237

237238
g := &Graph{

‎internal/mode/static/state/graph/policies.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ func processPolicies(
161161
validator validation.PolicyValidator,
162162
gateways processedGateways,
163163
routes map[RouteKey]*L7Route,
164-
policySettings *policies.GlobalPolicySettings,
164+
policyValidationCtx *policies.ValidationContext,
165165
) map[PolicyKey]*Policy {
166166
if len(policies) == 0 || gateways.Winner == nil {
167167
return nil
@@ -202,7 +202,7 @@ func processPolicies(
202202
continue
203203
}
204204

205-
conds := validator.Validate(policy, policySettings)
205+
conds := validator.Validate(policy, policyValidationCtx)
206206

207207
processedPolicies[key] = &Policy{
208208
Source: policy,

‎internal/mode/static/state/graph/policies_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -622,7 +622,7 @@ func TestProcessPolicies(t *testing.T) {
622622
validator: &policiesfakes.FakeValidator{
623623
ValidateStub: func(
624624
policy policies.Policy,
625-
_ *policies.GlobalPolicySettings,
625+
_ *policies.ValidationContext,
626626
) []conditions.Condition {
627627
if policy.GetName() == "pol1" {
628628
return []conditions.Condition{staticConds.NewPolicyInvalid("invalid error")}

0 commit comments

Comments
 (0)