Skip to content

Commit

Permalink
Add unit tests; comments
Browse files Browse the repository at this point in the history
  • Loading branch information
sjberman committed May 29, 2024
1 parent 3c522c3 commit 5c23a2c
Show file tree
Hide file tree
Showing 19 changed files with 138 additions and 60 deletions.
2 changes: 1 addition & 1 deletion internal/mode/static/policies/clientsettings/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ keepalive_timeout {{ .KeepAlive.Timeout.Server }};
`

// Generate generates configuration as []byte for a ClientSettingsPolicy.
func Generate(policy policies.Policy, _ *policies.GlobalPolicySettings) []byte {
func Generate(policy policies.Policy, _ *policies.GlobalSettings) []byte {
csp := helpers.MustCastObject[*ngfAPI.ClientSettingsPolicy](policy)

return helpers.MustExecuteTemplate(tmpl, csp.Spec)
Expand Down
2 changes: 1 addition & 1 deletion internal/mode/static/policies/clientsettings/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func NewValidator(genericValidator validation.GenericValidator) *Validator {
}

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

targetRefPath := field.NewPath("spec").Child("targetRef")
Expand Down
8 changes: 4 additions & 4 deletions internal/mode/static/policies/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ import (
)

// GenerateFunc generates config as []byte for an NGF Policy.
type GenerateFunc func(policy Policy, globalSettings *GlobalPolicySettings) []byte
type GenerateFunc func(policy Policy, globalSettings *GlobalSettings) []byte

// Validator validates an NGF Policy.
//
//counterfeiter:generate . Validator
type Validator interface {
// Validate validates an NGF Policy.
Validate(policy Policy, globalSettings *GlobalPolicySettings) []conditions.Condition
Validate(policy Policy, globalSettings *GlobalSettings) []conditions.Condition
// Conflicts returns true if the two Policies conflict.
Conflicts(a, b Policy) bool
}
Expand Down Expand Up @@ -62,7 +62,7 @@ func NewManager(
}

// Generate generates config for the policy as a byte array.
func (m *Manager) Generate(policy Policy, globalSettings *GlobalPolicySettings) []byte {
func (m *Manager) Generate(policy Policy, globalSettings *GlobalSettings) []byte {
gvk := m.mustExtractGVK(policy)

generate, ok := m.generators[gvk]
Expand All @@ -74,7 +74,7 @@ func (m *Manager) Generate(policy Policy, globalSettings *GlobalPolicySettings)
}

// Validate validates the policy.
func (m *Manager) Validate(policy Policy, globalSettings *GlobalPolicySettings) []conditions.Condition {
func (m *Manager) Validate(policy Policy, globalSettings *GlobalSettings) []conditions.Condition {
gvk := m.mustExtractGVK(policy)

validator, ok := m.validators[gvk]
Expand Down
8 changes: 4 additions & 4 deletions internal/mode/static/policies/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,24 +42,24 @@ var _ = Describe("Policy Manager", func() {
mustExtractGVK,
policies.ManagerConfig{
Validator: &policiesfakes.FakeValidator{
ValidateStub: func(_ policies.Policy, _ *policies.GlobalPolicySettings) []conditions.Condition {
ValidateStub: func(_ policies.Policy, _ *policies.GlobalSettings) []conditions.Condition {
return []conditions.Condition{staticConds.NewPolicyInvalid("apple error")}
},
ConflictsStub: func(_ policies.Policy, _ policies.Policy) bool { return true },
},
Generator: func(_ policies.Policy, _ *policies.GlobalPolicySettings) []byte {
Generator: func(_ policies.Policy, _ *policies.GlobalSettings) []byte {
return []byte("apple")
},
GVK: appleGVK,
},
policies.ManagerConfig{
Validator: &policiesfakes.FakeValidator{
ValidateStub: func(_ policies.Policy, _ *policies.GlobalPolicySettings) []conditions.Condition {
ValidateStub: func(_ policies.Policy, _ *policies.GlobalSettings) []conditions.Condition {
return []conditions.Condition{staticConds.NewPolicyInvalid("orange error")}
},
ConflictsStub: func(_ policies.Policy, _ policies.Policy) bool { return false },
},
Generator: func(_ policies.Policy, _ *policies.GlobalPolicySettings) []byte {
Generator: func(_ policies.Policy, _ *policies.GlobalSettings) []byte {
return []byte("orange")
},
GVK: orangeGVK,
Expand Down
2 changes: 1 addition & 1 deletion internal/mode/static/policies/observability/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ otel_span_attr "{{ $attr.Key }}" "{{ $attr.Value }}";
`

// Generate generates configuration as []byte for an ObservabilityPolicy.
func Generate(policy policies.Policy, globalSettings *policies.GlobalPolicySettings) []byte {
func Generate(policy policies.Policy, globalSettings *policies.GlobalSettings) []byte {
obs := helpers.MustCastObject[*ngfAPI.ObservabilityPolicy](policy)

var strategy string
Expand Down
6 changes: 3 additions & 3 deletions internal/mode/static/policies/observability/generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestGenerate(t *testing.T) {
tests := []struct {
name string
policy policies.Policy
globalSettings *policies.GlobalPolicySettings
globalSettings *policies.GlobalSettings
expStrings []string
}{
{
Expand Down Expand Up @@ -138,7 +138,7 @@ func TestGenerate(t *testing.T) {
Tracing: &ngfAPI.Tracing{},
},
},
globalSettings: &policies.GlobalPolicySettings{
globalSettings: &policies.GlobalSettings{
TracingSpanAttributes: []ngfAPI.SpanAttribute{
{Key: "test-global-key", Value: "test-global-value"},
},
Expand All @@ -162,7 +162,7 @@ func TestGenerate(t *testing.T) {
},
},
},
globalSettings: &policies.GlobalPolicySettings{
globalSettings: &policies.GlobalSettings{
TracingSpanAttributes: []ngfAPI.SpanAttribute{
{Key: "test-global-key", Value: "test-global-value"},
},
Expand Down
2 changes: 1 addition & 1 deletion internal/mode/static/policies/observability/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func NewValidator(genericValidator validation.GenericValidator) *Validator {
// Validate validates the spec of an ObservabilityPolicy.
func (v *Validator) Validate(
policy policies.Policy,
globalSettings *policies.GlobalPolicySettings,
globalSettings *policies.GlobalSettings,
) []conditions.Condition {
obs := helpers.MustCastObject[*ngfAPI.ObservabilityPolicy](policy)

Expand Down
8 changes: 4 additions & 4 deletions internal/mode/static/policies/observability/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,15 @@ func createModifiedPolicy(mod policyModFunc) *ngfAPI.ObservabilityPolicy {
}

func TestValidator_Validate(t *testing.T) {
globalSettings := &policies.GlobalPolicySettings{
globalSettings := &policies.GlobalSettings{
NginxProxyValid: true,
TelemetryEnabled: true,
}

tests := []struct {
name string
policy *ngfAPI.ObservabilityPolicy
globalSettings *policies.GlobalPolicySettings
globalSettings *policies.GlobalSettings
expConditions []conditions.Condition
}{
{
Expand All @@ -73,15 +73,15 @@ func TestValidator_Validate(t *testing.T) {
{
name: "validation context is invalid",
policy: createValidPolicy(),
globalSettings: &policies.GlobalPolicySettings{NginxProxyValid: false},
globalSettings: &policies.GlobalSettings{NginxProxyValid: false},
expConditions: []conditions.Condition{
staticConds.NewPolicyNotAcceptedNginxProxyNotSet(staticConds.PolicyMessageNginxProxyInvalid),
},
},
{
name: "telemetry is not enabled",
policy: createValidPolicy(),
globalSettings: &policies.GlobalPolicySettings{NginxProxyValid: true, TelemetryEnabled: false},
globalSettings: &policies.GlobalSettings{NginxProxyValid: true, TelemetryEnabled: false},
expConditions: []conditions.Condition{
staticConds.NewPolicyNotAcceptedNginxProxyNotSet(staticConds.PolicyMessageTelemetryNotEnabled),
},
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions internal/mode/static/policies/policiesfakes/fake_validator.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 8 additions & 5 deletions internal/mode/static/policies/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,18 @@ type Policy interface {
//
//counterfeiter:generate . ConfigGenerator
type ConfigGenerator interface {
Generate(policy Policy, globalSettings *GlobalPolicySettings) []byte
Generate(policy Policy, globalSettings *GlobalSettings) []byte
}

// GlobalPolicySettings contains global settings from the current state of the graph that may be
// GlobalSettings contains global settings from the current state of the graph that may be
// needed for policy validation or generation if certain policies rely on those global settings.
type GlobalPolicySettings struct {
type GlobalSettings struct {
// TracingSpanAttributes contain the attributes specified in the NginxProxy resource.
TracingSpanAttributes []ngfAPI.SpanAttribute
NginxProxyValid bool
TelemetryEnabled bool
// NginxProxyValid is whether or not the NginxProxy resource is valid.
NginxProxyValid bool
// TelemetryEnabled is whether or not telemetry is enabled in the NginxProxy resource.
TelemetryEnabled bool
}

// ValidateTargetRef validates a policy's targetRef for the proper group and kind.
Expand Down
10 changes: 5 additions & 5 deletions internal/mode/static/state/dataplane/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ func buildServers(g *graph.Graph, generator policies.ConfigGenerator) (http, ssl
rulesForProtocol[l.Source.Protocol][l.Source.Port] = rules
}

rules.upsertListener(l, g.GlobalPolicySettings)
rules.upsertListener(l, g.GlobalSettings)
}
}

Expand All @@ -227,7 +227,7 @@ func buildServers(g *graph.Graph, generator policies.ConfigGenerator) (http, ssl

httpServers, sslServers := httpRules.buildServers(), sslRules.buildServers()

additions := buildAdditions(g.Gateway.Policies, g.GlobalPolicySettings, generator)
additions := buildAdditions(g.Gateway.Policies, g.GlobalSettings, generator)

for i := range httpServers {
httpServers[i].Additions = additions
Expand Down Expand Up @@ -281,7 +281,7 @@ func newHostPathRules(generator policies.ConfigGenerator) *hostPathRules {
}
}

func (hpr *hostPathRules) upsertListener(l *graph.Listener, globalSettings *policies.GlobalPolicySettings) {
func (hpr *hostPathRules) upsertListener(l *graph.Listener, globalSettings *policies.GlobalSettings) {
hpr.listenersExist = true
hpr.port = int32(l.Source.Port)

Expand All @@ -301,7 +301,7 @@ func (hpr *hostPathRules) upsertListener(l *graph.Listener, globalSettings *poli
func (hpr *hostPathRules) upsertRoute(
route *graph.L7Route,
listener *graph.Listener,
globalSettings *policies.GlobalPolicySettings,
globalSettings *policies.GlobalSettings,
) {
var hostnames []string
GRPC := route.RouteType == graph.RouteTypeGRPC
Expand Down Expand Up @@ -674,7 +674,7 @@ func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig {

func buildAdditions(
policies []*graph.Policy,
globalSettings *policies.GlobalPolicySettings,
globalSettings *policies.GlobalSettings,
generator policies.ConfigGenerator,
) []Addition {
if len(policies) == 0 {
Expand Down
Loading

0 comments on commit 5c23a2c

Please sign in to comment.