diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 96f59c6f4f..11fbf20937 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -171,7 +171,7 @@ jobs: fail-build: false - name: Upload scan result to GitHub Security tab - uses: github/codeql-action/upload-sarif@17a820bf2e43b47be2c72b39cc905417bc1ab6d0 # v3.28.6 + uses: github/codeql-action/upload-sarif@dd746615b3b9d728a6a37ca2045b68ca76d4841a # v3.28.8 continue-on-error: true with: sarif_file: ${{ steps.scan.outputs.sarif }} diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index b4da70ec2c..911fc6d738 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -44,13 +44,13 @@ jobs: # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL - uses: github/codeql-action/init@17a820bf2e43b47be2c72b39cc905417bc1ab6d0 # v3.28.6 + uses: github/codeql-action/init@dd746615b3b9d728a6a37ca2045b68ca76d4841a # v3.28.8 with: languages: ${{ matrix.language }} build-mode: ${{ matrix.build-mode }} queries: security-and-quality - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@17a820bf2e43b47be2c72b39cc905417bc1ab6d0 # v3.28.6 + uses: github/codeql-action/analyze@dd746615b3b9d728a6a37ca2045b68ca76d4841a # v3.28.8 with: category: "/language:${{matrix.language}}" diff --git a/.github/workflows/scorecards.yml b/.github/workflows/scorecards.yml index 5dfc06300a..cd04836f61 100644 --- a/.github/workflows/scorecards.yml +++ b/.github/workflows/scorecards.yml @@ -60,6 +60,6 @@ jobs: # Upload the results to GitHub's code scanning dashboard. - name: "Upload to code-scanning" - uses: github/codeql-action/upload-sarif@17a820bf2e43b47be2c72b39cc905417bc1ab6d0 # v3.28.6 + uses: github/codeql-action/upload-sarif@dd746615b3b9d728a6a37ca2045b68ca76d4841a # v3.28.8 with: sarif_file: results.sarif diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index cbb5999323..37f7c8a803 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -27,7 +27,7 @@ repos: exclude: (^examples/|^docs/|.*_test.go$) - repo: https://github.com/gitleaks/gitleaks - rev: v8.23.2 + rev: v8.23.3 hooks: - id: gitleaks diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index 6292962ec2..ed4ea9ed3d 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -49,11 +49,21 @@ type NginxProxySpec struct { // // +optional Logging *NginxLogging `json:"logging,omitempty"` + // NginxPlus specifies NGINX Plus additional settings. + // + // +optional + NginxPlus *NginxPlus `json:"nginxPlus,omitempty"` // DisableHTTP2 defines if http2 should be disabled for all servers. // Default is false, meaning http2 will be enabled for all servers. + DisableHTTP2 bool `json:"disableHTTP2,omitempty"` +} + +// NginxPlus specifies NGINX Plus additional settings. These will only be applied if NGINX Plus is being used. +type NginxPlus struct { + // AllowedAddresses specifies IPAddresses or CIDR blocks to the allow list for accessing the NGINX Plus API. // // +optional - DisableHTTP2 bool `json:"disableHTTP2,omitempty"` + AllowedAddresses []NginxPlusAllowAddress `json:"allowedAddresses,omitempty"` } // Telemetry specifies the OpenTelemetry configuration. @@ -149,7 +159,7 @@ type RewriteClientIP struct { // +listType=map // +listMapKey=type // +kubebuilder:validation:MaxItems=16 - TrustedAddresses []Address `json:"trustedAddresses,omitempty"` + TrustedAddresses []RewriteClientIPAddress `json:"trustedAddresses,omitempty"` } // RewriteClientIPModeType defines how NGINX Gateway Fabric will determine the client's original IP address. @@ -183,28 +193,49 @@ const ( IPv6 IPFamilyType = "ipv6" ) -// Address is a struct that specifies address type and value. -type Address struct { +// RewriteClientIPAddress specifies the address type and value for a RewriteClientIP address. +type RewriteClientIPAddress struct { // Type specifies the type of address. - Type AddressType `json:"type"` + Type RewriteClientIPAddressType `json:"type"` // Value specifies the address value. Value string `json:"value"` } -// AddressType specifies the type of address. +// RewriteClientIPAddressType specifies the type of address. // +kubebuilder:validation:Enum=CIDR;IPAddress;Hostname -type AddressType string +type RewriteClientIPAddressType string const ( - // CIDRAddressType specifies that the address is a CIDR block. - CIDRAddressType AddressType = "CIDR" + // RewriteClientIPCIDRAddressType specifies that the address is a CIDR block. + RewriteClientIPCIDRAddressType RewriteClientIPAddressType = "CIDR" + + // RewriteClientIPIPAddressType specifies that the address is an IP address. + RewriteClientIPIPAddressType RewriteClientIPAddressType = "IPAddress" + + // RewriteClientIPHostnameAddressType specifies that the address is a Hostname. + RewriteClientIPHostnameAddressType RewriteClientIPAddressType = "Hostname" +) - // IPAddressType specifies that the address is an IP address. - IPAddressType AddressType = "IPAddress" +// NginxPlusAllowAddress specifies the address type and value for an NginxPlus allow address. +type NginxPlusAllowAddress struct { + // Type specifies the type of address. + Type NginxPlusAllowAddressType `json:"type"` + + // Value specifies the address value. + Value string `json:"value"` +} + +// NginxPlusAllowAddressType specifies the type of address. +// +kubebuilder:validation:Enum=CIDR;IPAddress +type NginxPlusAllowAddressType string + +const ( + // NginxPlusAllowCIDRAddressType specifies that the address is a CIDR block. + NginxPlusAllowCIDRAddressType NginxPlusAllowAddressType = "CIDR" - // HostnameAddressType specifies that the address is a Hostname. - HostnameAddressType AddressType = "Hostname" + // NginxPlusAllowIPAddressType specifies that the address is an IP address. + NginxPlusAllowIPAddressType NginxPlusAllowAddressType = "IPAddress" ) // NginxLogging defines logging related settings for NGINX. diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index 828faaa612..96100bed3f 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -10,21 +10,6 @@ import ( "sigs.k8s.io/gateway-api/apis/v1alpha2" ) -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Address) DeepCopyInto(out *Address) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Address. -func (in *Address) DeepCopy() *Address { - if in == nil { - return nil - } - out := new(Address) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClientBody) DeepCopyInto(out *ClientBody) { *out = *in @@ -353,6 +338,41 @@ func (in *NginxLogging) DeepCopy() *NginxLogging { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NginxPlus) DeepCopyInto(out *NginxPlus) { + *out = *in + if in.AllowedAddresses != nil { + in, out := &in.AllowedAddresses, &out.AllowedAddresses + *out = make([]NginxPlusAllowAddress, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxPlus. +func (in *NginxPlus) DeepCopy() *NginxPlus { + if in == nil { + return nil + } + out := new(NginxPlus) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NginxPlusAllowAddress) DeepCopyInto(out *NginxPlusAllowAddress) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxPlusAllowAddress. +func (in *NginxPlusAllowAddress) DeepCopy() *NginxPlusAllowAddress { + if in == nil { + return nil + } + out := new(NginxPlusAllowAddress) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NginxProxy) DeepCopyInto(out *NginxProxy) { *out = *in @@ -434,6 +454,11 @@ func (in *NginxProxySpec) DeepCopyInto(out *NginxProxySpec) { *out = new(NginxLogging) (*in).DeepCopyInto(*out) } + if in.NginxPlus != nil { + in, out := &in.NginxPlus, &out.NginxPlus + *out = new(NginxPlus) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxProxySpec. @@ -547,7 +572,7 @@ func (in *RewriteClientIP) DeepCopyInto(out *RewriteClientIP) { } if in.TrustedAddresses != nil { in, out := &in.TrustedAddresses, &out.TrustedAddresses - *out = make([]Address, len(*in)) + *out = make([]RewriteClientIPAddress, len(*in)) copy(*out, *in) } } @@ -562,6 +587,21 @@ func (in *RewriteClientIP) DeepCopy() *RewriteClientIP { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RewriteClientIPAddress) DeepCopyInto(out *RewriteClientIPAddress) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RewriteClientIPAddress. +func (in *RewriteClientIPAddress) DeepCopy() *RewriteClientIPAddress { + if in == nil { + return nil + } + out := new(RewriteClientIPAddress) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Snippet) DeepCopyInto(out *Snippet) { *out = *in diff --git a/charts/nginx-gateway-fabric/values.schema.json b/charts/nginx-gateway-fabric/values.schema.json index 048e4163cf..73a02d292d 100644 --- a/charts/nginx-gateway-fabric/values.schema.json +++ b/charts/nginx-gateway-fabric/values.schema.json @@ -93,6 +93,34 @@ "required": [], "type": "object" }, + "nginxPlus": { + "description": "NginxPlus specifies NGINX Plus additional settings.", + "properties": { + "allowedAddresses": { + "items": { + "properties": { + "type": { + "enum": [ + "CIDR", + "IPAddress" + ], + "required": [], + "type": "string" + }, + "value": { + "required": [], + "type": "string" + } + }, + "required": [] + }, + "required": [], + "type": "array" + } + }, + "required": [], + "type": "object" + }, "rewriteClientIP": { "description": "RewriteClientIP defines configuration for rewriting the client IP to the original client's IP.", "properties": { diff --git a/charts/nginx-gateway-fabric/values.yaml b/charts/nginx-gateway-fabric/values.yaml index 947212da97..2fe06c966c 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -244,6 +244,21 @@ nginx: # - crit # - alert # - emerg + # nginxPlus: + # type: object + # description: NginxPlus specifies NGINX Plus additional settings. + # properties: + # allowedAddresses: + # type: array + # items: + # properties: + # type: + # type: string + # enum: + # - CIDR + # - IPAddress + # value: + # type: string # @schema # -- The configuration for the data plane that is contained in the NginxProxy resource. config: {} diff --git a/cmd/gateway/initialize_test.go b/cmd/gateway/initialize_test.go index fcdc84990d..6f0f00ad8f 100644 --- a/cmd/gateway/initialize_test.go +++ b/cmd/gateway/initialize_test.go @@ -8,8 +8,8 @@ import ( "path/filepath" "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" - "sigs.k8s.io/controller-runtime/pkg/log/zap" "github.com/nginx/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginx/nginx-gateway-fabric/internal/mode/static/licensing/licensingfakes" @@ -27,7 +27,7 @@ func TestInitialize_OSS(t *testing.T) { ic := initializeConfig{ fileManager: fakeFileMgr, - logger: zap.New(), + logger: logr.Discard(), copy: copyFiles{ destDirName: "destDir", srcFileNames: []string{"src1", "src2"}, @@ -55,7 +55,7 @@ func TestInitialize_OSS_Error(t *testing.T) { ic := initializeConfig{ fileManager: fakeFileMgr, - logger: zap.New(), + logger: logr.Discard(), copy: copyFiles{ destDirName: "destDir", srcFileNames: []string{"src1", "src2"}, @@ -111,7 +111,7 @@ func TestInitialize_Plus(t *testing.T) { ic := initializeConfig{ fileManager: fakeFileMgr, - logger: zap.New(), + logger: logr.Discard(), collector: fakeCollector, fileGenerator: fakeGenerator, copy: copyFiles{ diff --git a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index b39ac21de3..b842c62214 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -83,6 +83,31 @@ spec: - emerg type: string type: object + nginxPlus: + description: NginxPlus specifies NGINX Plus additional settings. + properties: + allowedAddresses: + description: AllowedAddresses specifies IPAddresses or CIDR blocks + to the allow list for accessing the NGINX Plus API. + items: + description: NginxPlusAllowAddress specifies the address type + and value for an NginxPlus allow address. + properties: + type: + description: Type specifies the type of address. + enum: + - CIDR + - IPAddress + type: string + value: + description: Value specifies the address value. + type: string + required: + - type + - value + type: object + type: array + type: object rewriteClientIP: description: RewriteClientIP defines configuration for rewriting the client IP to the original client's IP. @@ -122,8 +147,8 @@ spec: Sets NGINX directive set_real_ip_from: https://nginx.org/en/docs/http/ngx_http_realip_module.html#set_real_ip_from This field is required if mode is set. items: - description: Address is a struct that specifies address type - and value. + description: RewriteClientIPAddress specifies the address type + and value for a RewriteClientIP address. properties: type: description: Type specifies the type of address. diff --git a/deploy/crds.yaml b/deploy/crds.yaml index a5ae02e961..f0547580f1 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -668,6 +668,31 @@ spec: - emerg type: string type: object + nginxPlus: + description: NginxPlus specifies NGINX Plus additional settings. + properties: + allowedAddresses: + description: AllowedAddresses specifies IPAddresses or CIDR blocks + to the allow list for accessing the NGINX Plus API. + items: + description: NginxPlusAllowAddress specifies the address type + and value for an NginxPlus allow address. + properties: + type: + description: Type specifies the type of address. + enum: + - CIDR + - IPAddress + type: string + value: + description: Value specifies the address value. + type: string + required: + - type + - value + type: object + type: array + type: object rewriteClientIP: description: RewriteClientIP defines configuration for rewriting the client IP to the original client's IP. @@ -707,8 +732,8 @@ spec: Sets NGINX directive set_real_ip_from: https://nginx.org/en/docs/http/ngx_http_realip_module.html#set_real_ip_from This field is required if mode is set. items: - description: Address is a struct that specifies address type - and value. + description: RewriteClientIPAddress specifies the address type + and value for a RewriteClientIP address. properties: type: description: Type specifies the type of address. diff --git a/go.mod b/go.mod index 8c00cae03b..aea2fa814c 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,7 @@ require ( github.com/prometheus/client_golang v1.20.5 github.com/prometheus/common v0.60.1 github.com/spf13/cobra v1.8.1 - github.com/spf13/pflag v1.0.5 + github.com/spf13/pflag v1.0.6 go.opentelemetry.io/otel v1.34.0 go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.34.0 go.uber.org/zap v1.27.0 diff --git a/go.sum b/go.sum index d15f4091ad..31fc06d5cd 100644 --- a/go.sum +++ b/go.sum @@ -115,8 +115,9 @@ github.com/sclevine/spec v1.4.0 h1:z/Q9idDcay5m5irkZ28M7PtQM4aOISzOpj4bUPkDee8= github.com/sclevine/spec v1.4.0/go.mod h1:LvpgJaFyvQzRvc1kaDs0bulYwzC70PbiYjC4QnFHkOM= github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM= github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y= -github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o= +github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= diff --git a/internal/framework/controller/register_test.go b/internal/framework/controller/register_test.go index e4a2fbc40c..115b737737 100644 --- a/internal/framework/controller/register_test.go +++ b/internal/framework/controller/register_test.go @@ -6,6 +6,7 @@ import ( "reflect" "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" "github.com/onsi/gomega/gcustom" gtypes "github.com/onsi/gomega/types" @@ -14,7 +15,6 @@ import ( "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" - "sigs.k8s.io/controller-runtime/pkg/log/zap" v1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1beta1" @@ -43,7 +43,7 @@ func TestRegister(t *testing.T) { mgr := &controllerfakes.FakeManager{} mgr.GetClientReturns(fake.NewClientBuilder().Build()) mgr.GetSchemeReturns(scheme) - mgr.GetLoggerReturns(zap.New()) + mgr.GetLoggerReturns(logr.Discard()) mgr.GetFieldIndexerReturns(indexer) return fakes{ diff --git a/internal/framework/events/events_test.go b/internal/framework/events/events_test.go index 6ed5ce8b54..12294e3d87 100644 --- a/internal/framework/events/events_test.go +++ b/internal/framework/events/events_test.go @@ -3,14 +3,14 @@ package events import ( "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" - "sigs.k8s.io/controller-runtime/pkg/log/zap" ) func TestEventLoop_SwapBatches(t *testing.T) { t.Parallel() g := NewWithT(t) - eventLoop := NewEventLoop(nil, zap.New(), nil, nil) + eventLoop := NewEventLoop(nil, logr.Discard(), nil, nil) eventLoop.currentBatch = EventBatch{ "event0", diff --git a/internal/framework/events/loop_test.go b/internal/framework/events/loop_test.go index 45560c4ce2..02f5a00b04 100644 --- a/internal/framework/events/loop_test.go +++ b/internal/framework/events/loop_test.go @@ -8,7 +8,6 @@ import ( "github.com/go-logr/logr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "sigs.k8s.io/controller-runtime/pkg/log/zap" "github.com/nginx/nginx-gateway-fabric/internal/framework/events" "github.com/nginx/nginx-gateway-fabric/internal/framework/events/eventsfakes" @@ -28,7 +27,7 @@ var _ = Describe("EventLoop", func() { eventCh = make(chan interface{}) fakePreparer = &eventsfakes.FakeFirstEventBatchPreparer{} - eventLoop = events.NewEventLoop(eventCh, zap.New(), fakeHandler, fakePreparer) + eventLoop = events.NewEventLoop(eventCh, logr.Discard(), fakeHandler, fakePreparer) errorCh = make(chan error) }) diff --git a/internal/framework/runnables/cronjob_test.go b/internal/framework/runnables/cronjob_test.go index 310ceef618..81be2a8c8c 100644 --- a/internal/framework/runnables/cronjob_test.go +++ b/internal/framework/runnables/cronjob_test.go @@ -5,8 +5,8 @@ import ( "testing" "time" + "github.com/go-logr/logr" . "github.com/onsi/gomega" - "sigs.k8s.io/controller-runtime/pkg/log/zap" ) func TestCronJob(t *testing.T) { @@ -26,7 +26,7 @@ func TestCronJob(t *testing.T) { cfg := CronJobConfig{ Worker: worker, - Logger: zap.New(), + Logger: logr.Discard(), Period: 1 * time.Millisecond, // 1ms is much smaller than timeout so the CronJob should run a few times ReadyCh: readyChannel, } @@ -58,7 +58,7 @@ func TestCronJob_ContextCanceled(t *testing.T) { cfg := CronJobConfig{ Worker: func(_ context.Context) {}, - Logger: zap.New(), + Logger: logr.Discard(), Period: 1 * time.Millisecond, // 1ms is much smaller than timeout so the CronJob should run a few times ReadyCh: readyChannel, } diff --git a/internal/framework/status/leader_aware_group_updater_test.go b/internal/framework/status/leader_aware_group_updater_test.go index c2bd5b702b..7f164afc05 100644 --- a/internal/framework/status/leader_aware_group_updater_test.go +++ b/internal/framework/status/leader_aware_group_updater_test.go @@ -3,13 +3,13 @@ package status import ( "context" + "github.com/go-logr/logr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - "sigs.k8s.io/controller-runtime/pkg/log/zap" v1 "sigs.k8s.io/gateway-api/apis/v1" ) @@ -46,7 +46,7 @@ var _ = Describe("LeaderAwareGroupUpdater", func() { ) BeforeAll(func() { - updater = NewLeaderAwareGroupUpdater(NewUpdater(k8sClient, zap.New())) + updater = NewLeaderAwareGroupUpdater(NewUpdater(k8sClient, logr.Discard())) for _, name := range allGCNames { gc := createGC(name) diff --git a/internal/framework/status/updater_retry_test.go b/internal/framework/status/updater_retry_test.go index 80466e6590..7768d78ad5 100644 --- a/internal/framework/status/updater_retry_test.go +++ b/internal/framework/status/updater_retry_test.go @@ -5,12 +5,12 @@ import ( "errors" "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log/zap" v1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/nginx/nginx-gateway-fabric/internal/framework/controller/controllerfakes" @@ -86,7 +86,7 @@ func TestNewRetryUpdateFunc(t *testing.T) { fakeStatusUpdater, types.NamespacedName{}, &v1.GatewayClass{}, - zap.New(), + logr.Discard(), func(client.Object) bool { return test.statusSetterReturns }, ) diff --git a/internal/framework/status/updater_test.go b/internal/framework/status/updater_test.go index e053517da7..b72faf4b3d 100644 --- a/internal/framework/status/updater_test.go +++ b/internal/framework/status/updater_test.go @@ -3,6 +3,7 @@ package status import ( "context" + "github.com/go-logr/logr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -10,7 +11,6 @@ import ( "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - "sigs.k8s.io/controller-runtime/pkg/log/zap" v1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/nginx/nginx-gateway-fabric/internal/framework/helpers" @@ -116,7 +116,7 @@ var _ = Describe("Updater", func() { ) BeforeAll(func() { - updater = NewUpdater(k8sClient, zap.New()) + updater = NewUpdater(k8sClient, logr.Discard()) for _, name := range gcNames { gc := createGC(name) diff --git a/internal/mode/provisioner/handler_test.go b/internal/mode/provisioner/handler_test.go index ee24ada683..97c870179e 100644 --- a/internal/mode/provisioner/handler_test.go +++ b/internal/mode/provisioner/handler_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/go-logr/logr" . "github.com/onsi/ginkgo/v2" v1 "k8s.io/api/apps/v1" apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -12,7 +13,6 @@ import ( "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - "sigs.k8s.io/controller-runtime/pkg/log/zap" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" . "github.com/onsi/gomega" @@ -60,7 +60,7 @@ var _ = Describe("handler", func() { return fakeTime } - statusUpdater = status.NewUpdater(k8sclient, zap.New()) + statusUpdater = status.NewUpdater(k8sclient, logr.Discard()) // Add GatewayClass CRD to the cluster crd = &metav1.PartialObjectMetadata{ @@ -114,7 +114,7 @@ var _ = Describe("handler", func() { Resource: crd, }, } - handler.HandleEventBatch(context.Background(), zap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) // Ensure GatewayClass is accepted @@ -152,7 +152,7 @@ var _ = Describe("handler", func() { }, } - handler.HandleEventBatch(context.Background(), zap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) depNsName := types.NamespacedName{ Namespace: "nginx-gateway", @@ -187,7 +187,7 @@ var _ = Describe("handler", func() { }, } - handler.HandleEventBatch(context.Background(), zap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) updatedGC := &gatewayv1.GatewayClass{} @@ -249,7 +249,7 @@ var _ = Describe("handler", func() { } handle := func() { - handler.HandleEventBatch(context.Background(), zap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) } Expect(handle).Should(Panic()) @@ -310,7 +310,7 @@ var _ = Describe("handler", func() { }, } - handler.HandleEventBatch(context.Background(), zap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) deps := &v1.DeploymentList{} err := k8sclient.List(context.Background(), deps) @@ -330,7 +330,7 @@ var _ = Describe("handler", func() { }, } - handler.HandleEventBatch(context.Background(), zap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) deps := &v1.DeploymentList{} @@ -359,7 +359,7 @@ var _ = Describe("handler", func() { }, } - handler.HandleEventBatch(context.Background(), zap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) deps := &v1.DeploymentList{} err := k8sclient.List(context.Background(), deps) @@ -392,7 +392,7 @@ var _ = Describe("handler", func() { }, } - handler.HandleEventBatch(context.Background(), zap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) unknownGC := &gatewayv1.GatewayClass{} err = k8sclient.Get(context.Background(), client.ObjectKeyFromObject(newGC), unknownGC) @@ -456,7 +456,7 @@ var _ = Describe("handler", func() { batch := []interface{}{e} handle := func() { - handler.HandleEventBatch(context.Background(), zap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) } Expect(handle).Should(Panic()) @@ -524,7 +524,7 @@ var _ = Describe("handler", func() { } handle := func() { - handler.HandleEventBatch(context.Background(), zap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) } Expect(handle).Should(Panic()) @@ -545,7 +545,7 @@ var _ = Describe("handler", func() { } handle := func() { - handler.HandleEventBatch(context.Background(), zap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) } Expect(handle).Should(Panic()) diff --git a/internal/mode/static/config_updater_test.go b/internal/mode/static/config_updater_test.go index 7b5f77029b..440f7dde22 100644 --- a/internal/mode/static/config_updater_test.go +++ b/internal/mode/static/config_updater_test.go @@ -5,10 +5,10 @@ import ( "fmt" "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" - "sigs.k8s.io/controller-runtime/pkg/log/zap" ngfAPI "github.com/nginx/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginx/nginx-gateway-fabric/internal/framework/helpers" @@ -33,7 +33,7 @@ func TestUpdateControlPlane(t *testing.T) { }, } - logger := zap.New() + logger := logr.Discard() nsname := types.NamespacedName{Namespace: "test", Name: "test"} tests := []struct { diff --git a/internal/mode/static/handler.go b/internal/mode/static/handler.go index e7be8e5d1d..54dde7ade8 100644 --- a/internal/mode/static/handler.go +++ b/internal/mode/static/handler.go @@ -174,7 +174,7 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Log return case state.EndpointsOnlyChange: h.version++ - cfg := dataplane.BuildConfiguration(ctx, gr, h.cfg.serviceResolver, h.version) + cfg := dataplane.BuildConfiguration(ctx, gr, h.cfg.serviceResolver, h.version, h.cfg.plus) depCtx, getErr := h.getDeploymentContext(ctx) if getErr != nil { logger.Error(getErr, "error getting deployment context for usage reporting") @@ -190,7 +190,7 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Log } case state.ClusterStateChange: h.version++ - cfg := dataplane.BuildConfiguration(ctx, gr, h.cfg.serviceResolver, h.version) + cfg := dataplane.BuildConfiguration(ctx, gr, h.cfg.serviceResolver, h.version, h.cfg.plus) depCtx, getErr := h.getDeploymentContext(ctx) if getErr != nil { logger.Error(getErr, "error getting deployment context for usage reporting") diff --git a/internal/mode/static/handler_test.go b/internal/mode/static/handler_test.go index 7a315b5c2d..5774487650 100644 --- a/internal/mode/static/handler_test.go +++ b/internal/mode/static/handler_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" + "github.com/go-logr/logr" ngxclient "github.com/nginxinc/nginx-plus-go-client/client" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -15,7 +16,6 @@ import ( "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - ctlrZap "sigs.k8s.io/controller-runtime/pkg/log/zap" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" ngfAPI "github.com/nginx/nginx-gateway-fabric/apis/v1alpha1" @@ -154,7 +154,7 @@ var _ = Describe("eventHandler", func() { e := &events.UpsertEvent{Resource: &gatewayv1.HTTPRoute{}} batch := []interface{}{e} - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) dcfg := dataplane.GetDefaultConfiguration(&graph.Graph{}, 1) @@ -170,7 +170,7 @@ var _ = Describe("eventHandler", func() { } batch := []interface{}{e} - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) dcfg := dataplane.GetDefaultConfiguration(&graph.Graph{}, 1) @@ -189,12 +189,12 @@ var _ = Describe("eventHandler", func() { } batch := []interface{}{upsertEvent, deleteEvent} - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) checkUpsertEventExpectations(upsertEvent) checkDeleteEventExpectations(deleteEvent) - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) dcfg := dataplane.GetDefaultConfiguration(&graph.Graph{}, 2) Expect(helpers.Diff(handler.GetLatestConfiguration(), &dcfg)).To(BeEmpty()) @@ -239,7 +239,7 @@ var _ = Describe("eventHandler", func() { expectedReqsCount = 2 } - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) Expect(fakeStatusUpdater.UpdateGroupCallCount()).To(Equal(2)) @@ -275,7 +275,7 @@ var _ = Describe("eventHandler", func() { It("handles a valid config", func() { batch := []interface{}{&events.UpsertEvent{Resource: cfg(ngfAPI.ControllerLogLevelError)}} - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) Expect(handler.GetLatestConfiguration()).To(BeNil()) @@ -290,7 +290,7 @@ var _ = Describe("eventHandler", func() { It("handles an invalid config", func() { batch := []interface{}{&events.UpsertEvent{Resource: cfg(ngfAPI.ControllerLogLevel("invalid"))}} - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) Expect(handler.GetLatestConfiguration()).To(BeNil()) @@ -318,7 +318,7 @@ var _ = Describe("eventHandler", func() { }, }, } - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) Expect(handler.GetLatestConfiguration()).To(BeNil()) @@ -351,7 +351,7 @@ var _ = Describe("eventHandler", func() { }} batch := []interface{}{e} - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) Expect(fakeStatusUpdater.UpdateGroupCallCount()).To(BeZero()) @@ -359,7 +359,7 @@ var _ = Describe("eventHandler", func() { batch = []interface{}{de} Expect(fakeK8sClient.Delete(context.Background(), createService(notNginxGatewayServiceName))).To(Succeed()) - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) Expect(handler.GetLatestConfiguration()).To(BeNil()) @@ -375,7 +375,7 @@ var _ = Describe("eventHandler", func() { }} batch := []interface{}{e} - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) Expect(handler.GetLatestConfiguration()).To(BeNil()) Expect(fakeStatusUpdater.UpdateGroupCallCount()).To(Equal(1)) @@ -395,7 +395,7 @@ var _ = Describe("eventHandler", func() { batch := []interface{}{e} Expect(fakeK8sClient.Delete(context.Background(), createService(nginxGatewayServiceName))).To(Succeed()) - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) Expect(handler.GetLatestConfiguration()).To(BeNil()) Expect(fakeStatusUpdater.UpdateGroupCallCount()).To(Equal(1)) @@ -439,9 +439,10 @@ var _ = Describe("eventHandler", func() { It("should call the NGINX Plus API", func() { handler.cfg.plus = true - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) dcfg := dataplane.GetDefaultConfiguration(&graph.Graph{}, 1) + dcfg.NginxPlus = dataplane.NginxPlus{AllowedAddresses: []string{"127.0.0.1"}} Expect(helpers.Diff(handler.GetLatestConfiguration(), &dcfg)).To(BeEmpty()) Expect(fakeGenerator.GenerateCallCount()).To(Equal(0)) @@ -452,7 +453,7 @@ var _ = Describe("eventHandler", func() { When("not running NGINX Plus", func() { It("should not call the NGINX Plus API", func() { - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) dcfg := dataplane.GetDefaultConfiguration(&graph.Graph{}, 1) Expect(helpers.Diff(handler.GetLatestConfiguration(), &dcfg)).To(BeEmpty()) @@ -542,7 +543,7 @@ var _ = Describe("eventHandler", func() { fakeProcessor.ProcessReturns(state.ClusterStateChange, &graph.Graph{}) Expect(handler.cfg.nginxConfiguredOnStartChecker.readyCheck(nil)).ToNot(Succeed()) - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) dcfg := dataplane.GetDefaultConfiguration(&graph.Graph{}, 1) Expect(helpers.Diff(handler.GetLatestConfiguration(), &dcfg)).To(BeEmpty()) @@ -558,7 +559,7 @@ var _ = Describe("eventHandler", func() { readyChannel := handler.cfg.nginxConfiguredOnStartChecker.getReadyCh() Expect(handler.cfg.nginxConfiguredOnStartChecker.readyCheck(nil)).ToNot(Succeed()) - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) Expect(handler.GetLatestConfiguration()).To(BeNil()) @@ -575,14 +576,14 @@ var _ = Describe("eventHandler", func() { fakeProcessor.ProcessReturns(state.ClusterStateChange, &graph.Graph{}) fakeNginxRuntimeMgr.ReloadReturns(errors.New("reload error")) - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) Expect(handler.cfg.nginxConfiguredOnStartChecker.readyCheck(nil)).ToNot(Succeed()) // now send an update with no changes; should still return an error fakeProcessor.ProcessReturns(state.NoChange, &graph.Graph{}) - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) Expect(handler.cfg.nginxConfiguredOnStartChecker.readyCheck(nil)).ToNot(Succeed()) @@ -590,7 +591,7 @@ var _ = Describe("eventHandler", func() { fakeProcessor.ProcessReturns(state.ClusterStateChange, &graph.Graph{}) fakeNginxRuntimeMgr.ReloadReturns(nil) - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) dcfg := dataplane.GetDefaultConfiguration(&graph.Graph{}, 2) Expect(helpers.Diff(handler.GetLatestConfiguration(), &dcfg)).To(BeEmpty()) @@ -605,7 +606,7 @@ var _ = Describe("eventHandler", func() { handle := func() { batch := []interface{}{e} - handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) } Expect(handle).Should(Panic()) diff --git a/internal/mode/static/nginx/conf/nginx-plus.conf b/internal/mode/static/nginx/conf/nginx-plus.conf index 17ac6de4e3..b1651bb5dc 100644 --- a/internal/mode/static/nginx/conf/nginx-plus.conf +++ b/internal/mode/static/nginx/conf/nginx-plus.conf @@ -27,30 +27,6 @@ http { tcp_nopush on; server_tokens off; - - server { - listen 127.0.0.1:8765; - root /usr/share/nginx/html; - access_log off; - - allow 127.0.0.1; - deny all; - - location = /dashboard.html {} - - location /api { - api write=off; - } - } - - server { - listen unix:/var/run/nginx/nginx-plus-api.sock; - access_log off; - - location /api { - api write=on; - } - } } stream { diff --git a/internal/mode/static/nginx/config/generator.go b/internal/mode/static/nginx/config/generator.go index 13d664c28e..00b9500aea 100644 --- a/internal/mode/static/nginx/config/generator.go +++ b/internal/mode/static/nginx/config/generator.go @@ -58,6 +58,9 @@ const ( // mgmtIncludesFile is the path to the file containing the NGINX Plus mgmt config. mgmtIncludesFile = mainIncludesFolder + "/mgmt.conf" + + // nginxPlusConfigFile is the path to the file containing the NGINX Plus API config. + nginxPlusConfigFile = httpFolder + "/plus-api.conf" ) // ConfigFolders is a list of folders where NGINX configuration files are stored. @@ -199,6 +202,7 @@ func (g GeneratorImpl) getExecuteFuncs( g.executeStreamUpstreams, executeStreamMaps, executeVersion, + executePlusAPI, } } diff --git a/internal/mode/static/nginx/config/generator_test.go b/internal/mode/static/nginx/config/generator_test.go index 435314e881..79bb35a90f 100644 --- a/internal/mode/static/nginx/config/generator_test.go +++ b/internal/mode/static/nginx/config/generator_test.go @@ -5,9 +5,9 @@ import ( "sort" "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/types" - ctlrZap "sigs.k8s.io/controller-runtime/pkg/log/zap" "github.com/nginx/nginx-gateway-fabric/internal/framework/helpers" ngfConfig "github.com/nginx/nginx-gateway-fabric/internal/mode/static/config" @@ -132,6 +132,7 @@ func TestGenerate(t *testing.T) { graph.PlusReportClientSSLCertificate: []byte("cert"), graph.PlusReportClientSSLKey: []byte("key"), }, + NginxPlus: dataplane.NginxPlus{AllowedAddresses: []string{"127.0.0.3", "25.0.0.3", "24.0.0.3/32"}}, } g := NewWithT(t) @@ -139,12 +140,12 @@ func TestGenerate(t *testing.T) { generator := config.NewGeneratorImpl( plus, &ngfConfig.UsageReportConfig{Endpoint: "test-endpoint"}, - ctlrZap.New(), + logr.Discard(), ) files := generator.Generate(conf) - g.Expect(files).To(HaveLen(17)) + g.Expect(files).To(HaveLen(18)) arrange := func(i, j int) bool { return files[i].Path < files[j].Path } @@ -155,6 +156,7 @@ func TestGenerate(t *testing.T) { /etc/nginx/conf.d/config-version.conf /etc/nginx/conf.d/http.conf /etc/nginx/conf.d/matches.json + /etc/nginx/conf.d/plus-api.conf /etc/nginx/includes/http_snippet1.conf /etc/nginx/includes/http_snippet2.conf /etc/nginx/includes/main_snippet1.conf @@ -196,33 +198,45 @@ func TestGenerate(t *testing.T) { g.Expect(httpCfg).To(ContainSubstring("include /etc/nginx/includes/http_snippet2.conf;")) g.Expect(files[2].Path).To(Equal("/etc/nginx/conf.d/matches.json")) - g.Expect(files[2].Type).To(Equal(file.TypeRegular)) expString := "{}" g.Expect(string(files[2].Content)).To(Equal(expString)) + g.Expect(files[3].Path).To(Equal("/etc/nginx/conf.d/plus-api.conf")) + g.Expect(files[3].Type).To(Equal(file.TypeRegular)) + httpCfg = string(files[3].Content) + g.Expect(httpCfg).To(ContainSubstring("listen unix:/var/run/nginx/nginx-plus-api.sock;")) + g.Expect(httpCfg).To(ContainSubstring("access_log off;")) + g.Expect(httpCfg).To(ContainSubstring("listen 8765;")) + g.Expect(httpCfg).To(ContainSubstring("root /usr/share/nginx/html;")) + g.Expect(httpCfg).To(ContainSubstring("allow 127.0.0.3;")) + g.Expect(httpCfg).To(ContainSubstring("allow 25.0.0.3;")) + g.Expect(httpCfg).To(ContainSubstring("allow 24.0.0.3/32;")) + g.Expect(httpCfg).To(ContainSubstring("deny all;")) + g.Expect(httpCfg).To(ContainSubstring("location = /dashboard.html {}")) + // snippet include files // content is not checked in this test. - g.Expect(files[3].Path).To(Equal("/etc/nginx/includes/http_snippet1.conf")) - g.Expect(files[4].Path).To(Equal("/etc/nginx/includes/http_snippet2.conf")) - g.Expect(files[5].Path).To(Equal("/etc/nginx/includes/main_snippet1.conf")) - g.Expect(files[6].Path).To(Equal("/etc/nginx/includes/main_snippet2.conf")) + g.Expect(files[4].Path).To(Equal("/etc/nginx/includes/http_snippet1.conf")) + g.Expect(files[5].Path).To(Equal("/etc/nginx/includes/http_snippet2.conf")) + g.Expect(files[6].Path).To(Equal("/etc/nginx/includes/main_snippet1.conf")) + g.Expect(files[7].Path).To(Equal("/etc/nginx/includes/main_snippet2.conf")) - g.Expect(files[7].Path).To(Equal("/etc/nginx/main-includes/deployment_ctx.json")) - deploymentCtx := string(files[7].Content) + g.Expect(files[8].Path).To(Equal("/etc/nginx/main-includes/deployment_ctx.json")) + deploymentCtx := string(files[8].Content) g.Expect(deploymentCtx).To(ContainSubstring("\"integration\":\"ngf\"")) g.Expect(deploymentCtx).To(ContainSubstring("\"cluster_id\":\"test-uid\"")) g.Expect(deploymentCtx).To(ContainSubstring("\"installation_id\":\"test-uid-replicaSet\"")) g.Expect(deploymentCtx).To(ContainSubstring("\"cluster_node_count\":1")) - g.Expect(files[8].Path).To(Equal("/etc/nginx/main-includes/main.conf")) - mainConfStr := string(files[8].Content) + g.Expect(files[9].Path).To(Equal("/etc/nginx/main-includes/main.conf")) + mainConfStr := string(files[9].Content) g.Expect(mainConfStr).To(ContainSubstring("load_module modules/ngx_otel_module.so;")) g.Expect(mainConfStr).To(ContainSubstring("include /etc/nginx/includes/main_snippet1.conf;")) g.Expect(mainConfStr).To(ContainSubstring("include /etc/nginx/includes/main_snippet2.conf;")) - g.Expect(files[9].Path).To(Equal("/etc/nginx/main-includes/mgmt.conf")) - mgmtConf := string(files[9].Content) + g.Expect(files[10].Path).To(Equal("/etc/nginx/main-includes/mgmt.conf")) + mgmtConf := string(files[10].Content) g.Expect(mgmtConf).To(ContainSubstring("usage_report endpoint=test-endpoint")) g.Expect(mgmtConf).To(ContainSubstring("license_token /etc/nginx/secrets/license.jwt")) g.Expect(mgmtConf).To(ContainSubstring("deployment_context /etc/nginx/main-includes/deployment_ctx.json")) @@ -230,31 +244,31 @@ func TestGenerate(t *testing.T) { g.Expect(mgmtConf).To(ContainSubstring("ssl_certificate /etc/nginx/secrets/mgmt-tls.crt")) g.Expect(mgmtConf).To(ContainSubstring("ssl_certificate_key /etc/nginx/secrets/mgmt-tls.key")) - g.Expect(files[10].Path).To(Equal("/etc/nginx/secrets/license.jwt")) - g.Expect(string(files[10].Content)).To(Equal("license")) + g.Expect(files[11].Path).To(Equal("/etc/nginx/secrets/license.jwt")) + g.Expect(string(files[11].Content)).To(Equal("license")) - g.Expect(files[11].Path).To(Equal("/etc/nginx/secrets/mgmt-ca.crt")) - g.Expect(string(files[11].Content)).To(Equal("ca")) + g.Expect(files[12].Path).To(Equal("/etc/nginx/secrets/mgmt-ca.crt")) + g.Expect(string(files[12].Content)).To(Equal("ca")) - g.Expect(files[12].Path).To(Equal("/etc/nginx/secrets/mgmt-tls.crt")) - g.Expect(string(files[12].Content)).To(Equal("cert")) + g.Expect(files[13].Path).To(Equal("/etc/nginx/secrets/mgmt-tls.crt")) + g.Expect(string(files[13].Content)).To(Equal("cert")) - g.Expect(files[13].Path).To(Equal("/etc/nginx/secrets/mgmt-tls.key")) - g.Expect(string(files[13].Content)).To(Equal("key")) + g.Expect(files[14].Path).To(Equal("/etc/nginx/secrets/mgmt-tls.key")) + g.Expect(string(files[14].Content)).To(Equal("key")) - g.Expect(files[14].Path).To(Equal("/etc/nginx/secrets/test-certbundle.crt")) - certBundle := string(files[14].Content) + g.Expect(files[15].Path).To(Equal("/etc/nginx/secrets/test-certbundle.crt")) + certBundle := string(files[15].Content) g.Expect(certBundle).To(Equal("test-cert")) - g.Expect(files[15]).To(Equal(file.File{ + g.Expect(files[16]).To(Equal(file.File{ Type: file.TypeSecret, Path: "/etc/nginx/secrets/test-keypair.pem", Content: []byte("test-cert\ntest-key"), })) - g.Expect(files[16].Path).To(Equal("/etc/nginx/stream-conf.d/stream.conf")) - g.Expect(files[16].Type).To(Equal(file.TypeRegular)) - streamCfg := string(files[16].Content) + g.Expect(files[17].Path).To(Equal("/etc/nginx/stream-conf.d/stream.conf")) + g.Expect(files[17].Type).To(Equal(file.TypeRegular)) + streamCfg := string(files[17].Content) g.Expect(streamCfg).To(ContainSubstring("listen unix:/var/run/nginx/app.example.com-443.sock")) g.Expect(streamCfg).To(ContainSubstring("listen 443")) g.Expect(streamCfg).To(ContainSubstring("app.example.com unix:/var/run/nginx/app.example.com-443.sock")) diff --git a/internal/mode/static/nginx/config/plus_api.go b/internal/mode/static/nginx/config/plus_api.go new file mode 100644 index 0000000000..9b1894fe30 --- /dev/null +++ b/internal/mode/static/nginx/config/plus_api.go @@ -0,0 +1,25 @@ +package config + +import ( + gotemplate "text/template" + + "github.com/nginx/nginx-gateway-fabric/internal/framework/helpers" + "github.com/nginx/nginx-gateway-fabric/internal/mode/static/state/dataplane" +) + +var plusAPITemplate = gotemplate.Must(gotemplate.New("plusAPI").Parse(plusAPITemplateText)) + +func executePlusAPI(conf dataplane.Configuration) []executeResult { + result := executeResult{ + dest: nginxPlusConfigFile, + } + // if AllowedAddresses is empty, it means that we are not running on nginx plus, and we don't want this generated + if conf.NginxPlus.AllowedAddresses != nil { + result = executeResult{ + dest: nginxPlusConfigFile, + data: helpers.MustExecuteTemplate(plusAPITemplate, conf.NginxPlus), + } + } + + return []executeResult{result} +} diff --git a/internal/mode/static/nginx/config/plus_api_template.go b/internal/mode/static/nginx/config/plus_api_template.go new file mode 100644 index 0000000000..94fb27ff39 --- /dev/null +++ b/internal/mode/static/nginx/config/plus_api_template.go @@ -0,0 +1,28 @@ +package config + +const plusAPITemplateText = ` +server { + listen unix:/var/run/nginx/nginx-plus-api.sock; + access_log off; + + location /api { + api write=on; + } +} + +server { + listen 8765; + root /usr/share/nginx/html; + access_log off; + {{ range $address := .AllowedAddresses }} + allow {{ $address }}; + {{- end }} + deny all; + + location = /dashboard.html {} + + location /api { + api write=off; + } +} +` diff --git a/internal/mode/static/nginx/config/plus_api_test.go b/internal/mode/static/nginx/config/plus_api_test.go new file mode 100644 index 0000000000..6afb79142a --- /dev/null +++ b/internal/mode/static/nginx/config/plus_api_test.go @@ -0,0 +1,63 @@ +package config + +import ( + "strings" + "testing" + + . "github.com/onsi/gomega" + + "github.com/nginx/nginx-gateway-fabric/internal/mode/static/state/dataplane" +) + +func TestExecutePlusAPI(t *testing.T) { + t.Parallel() + conf := dataplane.Configuration{ + NginxPlus: dataplane.NginxPlus{AllowedAddresses: []string{"127.0.0.1", "25.0.0.3"}}, + } + + g := NewWithT(t) + expSubStrings := map[string]int{ + "listen unix:/var/run/nginx/nginx-plus-api.sock;": 1, + "access_log off;": 2, + "api write=on;": 1, + "listen 8765;": 1, + "root /usr/share/nginx/html;": 1, + "allow 127.0.0.1;": 1, + "allow 25.0.0.3;": 1, + "deny all;": 1, + "location = /dashboard.html {}": 1, + "api write=off;": 1, + } + + for expSubStr, expCount := range expSubStrings { + res := executePlusAPI(conf) + g.Expect(res).To(HaveLen(1)) + g.Expect(expCount).To(Equal(strings.Count(string(res[0].data), expSubStr))) + } +} + +func TestExecutePlusAPI_EmptyNginxPlus(t *testing.T) { + t.Parallel() + conf := dataplane.Configuration{ + NginxPlus: dataplane.NginxPlus{}, + } + + g := NewWithT(t) + expSubStrings := map[string]int{ + "listen unix:/var/run/nginx/nginx-plus-api.sock;": 0, + "access_log off;": 0, + "api write=on;": 0, + "listen 8765;": 0, + "root /usr/share/nginx/html;": 0, + "allow 127.0.0.1;": 0, + "deny all;": 0, + "location = /dashboard.html {}": 0, + "api write=off;": 0, + } + + for expSubStr, expCount := range expSubStrings { + res := executePlusAPI(conf) + g.Expect(res).To(HaveLen(1)) + g.Expect(expCount).To(Equal(strings.Count(string(res[0].data), expSubStr))) + } +} diff --git a/internal/mode/static/nginx/file/manager_test.go b/internal/mode/static/nginx/file/manager_test.go index 1f0d5fa8a3..114b81c3dc 100644 --- a/internal/mode/static/nginx/file/manager_test.go +++ b/internal/mode/static/nginx/file/manager_test.go @@ -5,9 +5,9 @@ import ( "os" "path/filepath" + "github.com/go-logr/logr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "sigs.k8s.io/controller-runtime/pkg/log/zap" "github.com/nginx/nginx-gateway-fabric/internal/mode/static/nginx/file" "github.com/nginx/nginx-gateway-fabric/internal/mode/static/nginx/file/filefakes" @@ -60,7 +60,7 @@ var _ = Describe("EventHandler", func() { } BeforeAll(func() { - mgr = file.NewManagerImpl(zap.New(), file.NewStdLibOSFileManager()) + mgr = file.NewManagerImpl(logr.Discard(), file.NewStdLibOSFileManager()) tmpDir = GinkgoT().TempDir() regular1 = file.File{ @@ -119,7 +119,7 @@ var _ = Describe("EventHandler", func() { When("file does not exist", func() { It("should not error", func() { fakeOSMgr := &filefakes.FakeOSFileManager{} - mgr := file.NewManagerImpl(zap.New(), fakeOSMgr) + mgr := file.NewManagerImpl(logr.Discard(), fakeOSMgr) files := []file.File{ { @@ -138,7 +138,7 @@ var _ = Describe("EventHandler", func() { When("file type is not supported", func() { It("should panic", func() { - mgr := file.NewManagerImpl(zap.New(), nil) + mgr := file.NewManagerImpl(logr.Discard(), nil) files := []file.File{ { @@ -175,7 +175,7 @@ var _ = Describe("EventHandler", func() { DescribeTable( "should return error on file IO error", func(fakeOSMgr *filefakes.FakeOSFileManager) { - mgr := file.NewManagerImpl(zap.New(), fakeOSMgr) + mgr := file.NewManagerImpl(logr.Discard(), fakeOSMgr) // special case for Remove // to kick off removing, we need to successfully write files beforehand diff --git a/internal/mode/static/nginx/runtime/manager_test.go b/internal/mode/static/nginx/runtime/manager_test.go index e029a255ce..1c40c03513 100644 --- a/internal/mode/static/nginx/runtime/manager_test.go +++ b/internal/mode/static/nginx/runtime/manager_test.go @@ -8,10 +8,10 @@ import ( "testing" "time" + "github.com/go-logr/logr" ngxclient "github.com/nginxinc/nginx-plus-go-client/client" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "sigs.k8s.io/controller-runtime/pkg/log/zap" "github.com/nginx/nginx-gateway-fabric/internal/mode/static/nginx/runtime" "github.com/nginx/nginx-gateway-fabric/internal/mode/static/nginx/runtime/runtimefakes" @@ -19,10 +19,10 @@ import ( var _ = Describe("NGINX Runtime Manager", func() { It("returns whether or not we're using NGINX Plus", func() { - mgr := runtime.NewManagerImpl(nil, nil, zap.New(), nil, nil) + mgr := runtime.NewManagerImpl(nil, nil, logr.Discard(), nil, nil) Expect(mgr.IsPlus()).To(BeFalse()) - mgr = runtime.NewManagerImpl(&ngxclient.NginxClient{}, nil, zap.New(), nil, nil) + mgr = runtime.NewManagerImpl(&ngxclient.NginxClient{}, nil, logr.Discard(), nil, nil) Expect(mgr.IsPlus()).To(BeTrue()) }) @@ -53,7 +53,7 @@ var _ = Describe("NGINX Runtime Manager", func() { process = &runtimefakes.FakeProcessHandler{} metrics = &runtimefakes.FakeMetricsCollector{} verifyClient = &runtimefakes.FakeVerifyClient{} - manager = runtime.NewManagerImpl(ngxPlusClient, metrics, zap.New(), process, verifyClient) + manager = runtime.NewManagerImpl(ngxPlusClient, metrics, logr.Discard(), process, verifyClient) }) It("Is successful", func() { @@ -117,7 +117,7 @@ var _ = Describe("NGINX Runtime Manager", func() { When("MetricsCollector is nil", func() { It("panics", func() { metrics = nil - manager = runtime.NewManagerImpl(ngxPlusClient, metrics, zap.New(), process, verifyClient) + manager = runtime.NewManagerImpl(ngxPlusClient, metrics, logr.Discard(), process, verifyClient) reload := func() { err = manager.Reload(context.Background(), 0) @@ -132,7 +132,7 @@ var _ = Describe("NGINX Runtime Manager", func() { It("panics", func() { metrics = &runtimefakes.FakeMetricsCollector{} verifyClient = nil - manager = runtime.NewManagerImpl(ngxPlusClient, metrics, zap.New(), process, verifyClient) + manager = runtime.NewManagerImpl(ngxPlusClient, metrics, logr.Discard(), process, verifyClient) reload := func() { err = manager.Reload(context.Background(), 0) @@ -147,7 +147,7 @@ var _ = Describe("NGINX Runtime Manager", func() { When("running NGINX plus", func() { BeforeEach(func() { ngxPlusClient = &runtimefakes.FakeNginxPlusClient{} - manager = runtime.NewManagerImpl(ngxPlusClient, nil, zap.New(), nil, nil) + manager = runtime.NewManagerImpl(ngxPlusClient, nil, logr.Discard(), nil, nil) }) It("successfully updates HTTP server upstream", func() { @@ -263,7 +263,7 @@ var _ = Describe("NGINX Runtime Manager", func() { When("not running NGINX plus", func() { BeforeEach(func() { ngxPlusClient = nil - manager = runtime.NewManagerImpl(ngxPlusClient, nil, zap.New(), nil, nil) + manager = runtime.NewManagerImpl(ngxPlusClient, nil, logr.Discard(), nil, nil) }) It("should panic when fetching upstream servers", func() { diff --git a/internal/mode/static/state/change_processor_test.go b/internal/mode/static/state/change_processor_test.go index 7211a5f045..e899bdaa40 100644 --- a/internal/mode/static/state/change_processor_test.go +++ b/internal/mode/static/state/change_processor_test.go @@ -1,6 +1,7 @@ package state_test import ( + "github.com/go-logr/logr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/onsi/gomega/format" @@ -13,7 +14,6 @@ import ( "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log/zap" v1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1alpha2" "sigs.k8s.io/gateway-api/apis/v1alpha3" @@ -414,7 +414,7 @@ var _ = Describe("ChangeProcessor", func() { processor = state.NewChangeProcessorImpl(state.ChangeProcessorConfig{ GatewayCtlrName: controllerName, GatewayClassName: gcName, - Logger: zap.New(), + Logger: logr.Discard(), Validators: createAlwaysValidValidators(), MustExtractGVK: kinds.NewMustExtractGKV(createScheme()), }) @@ -2181,7 +2181,7 @@ var _ = Describe("ChangeProcessor", func() { processor = state.NewChangeProcessorImpl(state.ChangeProcessorConfig{ GatewayCtlrName: controllerName, GatewayClassName: gcName, - Logger: zap.New(), + Logger: logr.Discard(), Validators: createAlwaysValidValidators(), MustExtractGVK: kinds.NewMustExtractGKV(createScheme()), }) diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 33324dbf42..3b4760d331 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -33,9 +33,15 @@ func BuildConfiguration( g *graph.Graph, serviceResolver resolver.ServiceResolver, configVersion int, + plus bool, ) Configuration { if g.GatewayClass == nil || !g.GatewayClass.Valid || g.Gateway == nil { - return GetDefaultConfiguration(g, configVersion) + config := GetDefaultConfiguration(g, configVersion) + if plus { + config.NginxPlus = buildNginxPlus(g) + } + + return config } baseHTTPConfig := buildBaseHTTPConfig(g) @@ -50,6 +56,11 @@ func BuildConfiguration( baseHTTPConfig.IPFamily, ) + var nginxPlus NginxPlus + if plus { + nginxPlus = buildNginxPlus(g) + } + config := Configuration{ HTTPServers: httpServers, SSLServers: sslServers, @@ -63,6 +74,7 @@ func BuildConfiguration( Telemetry: buildTelemetry(g), BaseHTTPConfig: baseHTTPConfig, Logging: buildLogging(g), + NginxPlus: nginxPlus, MainSnippets: buildSnippetsForContext(g.SnippetsFilters, ngfAPIv1alpha1.NginxContextMain), AuxiliarySecrets: buildAuxiliarySecrets(g.PlusSecrets), } @@ -951,7 +963,7 @@ func buildPolicies(graphPolicies []*graph.Policy) []policies.Policy { return finalPolicies } -func convertAddresses(addresses []ngfAPIv1alpha1.Address) []string { +func convertAddresses(addresses []ngfAPIv1alpha1.RewriteClientIPAddress) []string { trustedAddresses := make([]string, len(addresses)) for i, addr := range addresses { trustedAddresses[i] = addr.Value @@ -986,10 +998,29 @@ func buildAuxiliarySecrets( return auxSecrets } +func buildNginxPlus(g *graph.Graph) NginxPlus { + nginxPlusSettings := NginxPlus{AllowedAddresses: []string{"127.0.0.1"}} + + ngfProxy := g.NginxProxy + if ngfProxy != nil && ngfProxy.Source.Spec.NginxPlus != nil { + if ngfProxy.Source.Spec.NginxPlus.AllowedAddresses != nil { + addresses := make([]string, 0, len(ngfProxy.Source.Spec.NginxPlus.AllowedAddresses)) + for _, addr := range ngfProxy.Source.Spec.NginxPlus.AllowedAddresses { + addresses = append(addresses, addr.Value) + } + + nginxPlusSettings.AllowedAddresses = addresses + } + } + + return nginxPlusSettings +} + func GetDefaultConfiguration(g *graph.Graph, configVersion int) Configuration { return Configuration{ Version: configVersion, Logging: buildLogging(g), + NginxPlus: NginxPlus{}, AuxiliarySecrets: buildAuxiliarySecrets(g.PlusSecrets), } } diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index a455d5c73e..0117afecdd 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -66,6 +66,7 @@ func getExpectedConfiguration() Configuration { Logging: Logging{ ErrorLevel: defaultErrorLogLevel, }, + NginxPlus: NginxPlus{}, } } @@ -865,6 +866,11 @@ func TestBuildConfiguration(t *testing.T) { Valid: true, } + defaultConfig := Configuration{ + Logging: Logging{ErrorLevel: defaultErrorLogLevel}, + NginxPlus: NginxPlus{}, + } + tests := []struct { graph *graph.Graph msg string @@ -1551,39 +1557,17 @@ func TestBuildConfiguration(t *testing.T) { { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { g.GatewayClass.Valid = false - g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr1): routeHR1, - }, - }) - g.Routes = map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr1): routeHR1, - } return g }), - expConf: Configuration{Logging: Logging{ErrorLevel: defaultErrorLogLevel}}, + expConf: defaultConfig, msg: "invalid gatewayclass", }, { graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { - g.GatewayClass.Valid = false - g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ - Name: "listener-80-1", - Source: listener80, - Valid: true, - Routes: map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr1): routeHR1, - }, - }) - g.Routes = map[graph.RouteKey]*graph.L7Route{ - graph.CreateRouteKey(hr1): routeHR1, - } + g.GatewayClass = nil return g }), - expConf: Configuration{Logging: Logging{ErrorLevel: defaultErrorLogLevel}}, + expConf: defaultConfig, msg: "missing gatewayclass", }, { @@ -1591,7 +1575,7 @@ func TestBuildConfiguration(t *testing.T) { g.Gateway = nil return g }), - expConf: Configuration{Logging: Logging{ErrorLevel: defaultErrorLogLevel}}, + expConf: defaultConfig, msg: "missing gateway", }, { @@ -2271,9 +2255,9 @@ func TestBuildConfiguration(t *testing.T) { Spec: ngfAPIv1alpha1.NginxProxySpec{ RewriteClientIP: &ngfAPIv1alpha1.RewriteClientIP{ SetIPRecursively: helpers.GetPointer(true), - TrustedAddresses: []ngfAPIv1alpha1.Address{ + TrustedAddresses: []ngfAPIv1alpha1.RewriteClientIPAddress{ { - Type: ngfAPIv1alpha1.CIDRAddressType, + Type: ngfAPIv1alpha1.RewriteClientIPCIDRAddressType, Value: "1.1.1.1/32", }, }, @@ -2366,6 +2350,40 @@ func TestBuildConfiguration(t *testing.T) { }), msg: "SnippetsFilters with main and http snippet", }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Source.ObjectMeta = metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + } + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }) + g.NginxProxy = &graph.NginxProxy{ + Valid: true, + Source: &ngfAPIv1alpha1.NginxProxy{ + Spec: ngfAPIv1alpha1.NginxProxySpec{ + NginxPlus: &ngfAPIv1alpha1.NginxPlus{ + AllowedAddresses: []ngfAPIv1alpha1.NginxPlusAllowAddress{ + {Type: ngfAPIv1alpha1.NginxPlusAllowIPAddressType, Value: "127.0.0.3"}, + {Type: ngfAPIv1alpha1.NginxPlusAllowIPAddressType, Value: "25.0.0.3"}, + }, + }, + }, + }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + return conf + }), + msg: "NginxProxy with NginxPlus allowed addresses configured but running on nginx oss", + }, } for _, test := range tests { @@ -2378,6 +2396,7 @@ func TestBuildConfiguration(t *testing.T) { test.graph, fakeResolver, 1, + false, ) g.Expect(result.BackendGroups).To(ConsistOf(test.expConf.BackendGroups)) @@ -2391,6 +2410,126 @@ func TestBuildConfiguration(t *testing.T) { g.Expect(result.Telemetry).To(Equal(test.expConf.Telemetry)) g.Expect(result.BaseHTTPConfig).To(Equal(test.expConf.BaseHTTPConfig)) g.Expect(result.Logging).To(Equal(test.expConf.Logging)) + g.Expect(result.NginxPlus).To(Equal(test.expConf.NginxPlus)) + }) + } +} + +func TestBuildConfiguration_Plus(t *testing.T) { + t.Parallel() + fooEndpoints := []resolver.Endpoint{ + { + Address: "10.0.0.0", + Port: 8080, + }, + } + + fakeResolver := &resolverfakes.FakeServiceResolver{} + fakeResolver.ResolveReturns(fooEndpoints, nil) + + listener80 := v1.Listener{ + Name: "listener-80-1", + Hostname: nil, + Port: 80, + Protocol: v1.HTTPProtocolType, + } + + defaultPlusConfig := Configuration{ + Logging: Logging{ErrorLevel: defaultErrorLogLevel}, + NginxPlus: NginxPlus{AllowedAddresses: []string{"127.0.0.1"}}, + } + + tests := []struct { + graph *graph.Graph + msg string + expConf Configuration + }{ + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Source.ObjectMeta = metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + } + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }) + g.NginxProxy = &graph.NginxProxy{ + Valid: true, + Source: &ngfAPIv1alpha1.NginxProxy{ + Spec: ngfAPIv1alpha1.NginxProxySpec{ + NginxPlus: &ngfAPIv1alpha1.NginxPlus{ + AllowedAddresses: []ngfAPIv1alpha1.NginxPlusAllowAddress{ + {Type: ngfAPIv1alpha1.NginxPlusAllowIPAddressType, Value: "127.0.0.3"}, + {Type: ngfAPIv1alpha1.NginxPlusAllowIPAddressType, Value: "25.0.0.3"}, + }, + }, + }, + }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + conf.NginxPlus = NginxPlus{AllowedAddresses: []string{"127.0.0.3", "25.0.0.3"}} + return conf + }), + msg: "NginxProxy with NginxPlus allowed addresses configured", + }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.GatewayClass.Valid = false + return g + }), + expConf: defaultPlusConfig, + msg: "invalid gatewayclass", + }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.GatewayClass = nil + return g + }), + expConf: defaultPlusConfig, + msg: "missing gatewayclass", + }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway = nil + return g + }), + expConf: defaultPlusConfig, + msg: "missing gateway", + }, + } + + for _, test := range tests { + t.Run(test.msg, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + result := BuildConfiguration( + context.TODO(), + test.graph, + fakeResolver, + 1, + true, + ) + + g.Expect(result.BackendGroups).To(ConsistOf(test.expConf.BackendGroups)) + g.Expect(result.Upstreams).To(ConsistOf(test.expConf.Upstreams)) + g.Expect(result.HTTPServers).To(ConsistOf(test.expConf.HTTPServers)) + g.Expect(result.SSLServers).To(ConsistOf(test.expConf.SSLServers)) + g.Expect(result.TLSPassthroughServers).To(ConsistOf(test.expConf.TLSPassthroughServers)) + g.Expect(result.SSLKeyPairs).To(Equal(test.expConf.SSLKeyPairs)) + g.Expect(result.Version).To(Equal(1)) + g.Expect(result.CertBundles).To(Equal(test.expConf.CertBundles)) + g.Expect(result.Telemetry).To(Equal(test.expConf.Telemetry)) + g.Expect(result.BaseHTTPConfig).To(Equal(test.expConf.BaseHTTPConfig)) + g.Expect(result.Logging).To(Equal(test.expConf.Logging)) + g.Expect(result.NginxPlus).To(Equal(test.expConf.NginxPlus)) }) } } @@ -3928,9 +4067,9 @@ func TestBuildRewriteIPSettings(t *testing.T) { Spec: ngfAPIv1alpha1.NginxProxySpec{ RewriteClientIP: &ngfAPIv1alpha1.RewriteClientIP{ Mode: helpers.GetPointer(ngfAPIv1alpha1.RewriteClientIPModeProxyProtocol), - TrustedAddresses: []ngfAPIv1alpha1.Address{ + TrustedAddresses: []ngfAPIv1alpha1.RewriteClientIPAddress{ { - Type: ngfAPIv1alpha1.CIDRAddressType, + Type: ngfAPIv1alpha1.RewriteClientIPCIDRAddressType, Value: "10.9.9.4/32", }, }, @@ -3955,9 +4094,9 @@ func TestBuildRewriteIPSettings(t *testing.T) { Spec: ngfAPIv1alpha1.NginxProxySpec{ RewriteClientIP: &ngfAPIv1alpha1.RewriteClientIP{ Mode: helpers.GetPointer(ngfAPIv1alpha1.RewriteClientIPModeXForwardedFor), - TrustedAddresses: []ngfAPIv1alpha1.Address{ + TrustedAddresses: []ngfAPIv1alpha1.RewriteClientIPAddress{ { - Type: ngfAPIv1alpha1.CIDRAddressType, + Type: ngfAPIv1alpha1.RewriteClientIPCIDRAddressType, Value: "76.89.90.11/24", }, }, @@ -3982,21 +4121,21 @@ func TestBuildRewriteIPSettings(t *testing.T) { Spec: ngfAPIv1alpha1.NginxProxySpec{ RewriteClientIP: &ngfAPIv1alpha1.RewriteClientIP{ Mode: helpers.GetPointer(ngfAPIv1alpha1.RewriteClientIPModeXForwardedFor), - TrustedAddresses: []ngfAPIv1alpha1.Address{ + TrustedAddresses: []ngfAPIv1alpha1.RewriteClientIPAddress{ { - Type: ngfAPIv1alpha1.CIDRAddressType, + Type: ngfAPIv1alpha1.RewriteClientIPCIDRAddressType, Value: "5.5.5.5/12", }, { - Type: ngfAPIv1alpha1.CIDRAddressType, + Type: ngfAPIv1alpha1.RewriteClientIPCIDRAddressType, Value: "1.1.1.1/26", }, { - Type: ngfAPIv1alpha1.CIDRAddressType, + Type: ngfAPIv1alpha1.RewriteClientIPCIDRAddressType, Value: "2.2.2.2/32", }, { - Type: ngfAPIv1alpha1.CIDRAddressType, + Type: ngfAPIv1alpha1.RewriteClientIPCIDRAddressType, Value: "3.3.3.3/24", }, }, @@ -4383,3 +4522,96 @@ func TestBuildAuxiliarySecrets(t *testing.T) { g.Expect(buildAuxiliarySecrets(secrets)).To(Equal(expSecrets)) } + +func TestBuildNginxPlus(t *testing.T) { + defaultNginxPlus := NginxPlus{AllowedAddresses: []string{"127.0.0.1"}} + + t.Parallel() + tests := []struct { + msg string + g *graph.Graph + expNginxPlus NginxPlus + }{ + { + msg: "NginxProxy is nil", + g: &graph.Graph{}, + expNginxPlus: defaultNginxPlus, + }, + { + msg: "NginxPlus default values are used when NginxProxy doesn't specify NginxPlus settings", + g: &graph.Graph{ + NginxProxy: &graph.NginxProxy{ + Valid: true, + Source: &ngfAPIv1alpha1.NginxProxy{ + Spec: ngfAPIv1alpha1.NginxProxySpec{}, + }, + }, + }, + expNginxPlus: defaultNginxPlus, + }, + { + msg: "NginxProxy specifies one allowed address", + g: &graph.Graph{ + NginxProxy: &graph.NginxProxy{ + Valid: true, + Source: &ngfAPIv1alpha1.NginxProxy{ + Spec: ngfAPIv1alpha1.NginxProxySpec{ + NginxPlus: &ngfAPIv1alpha1.NginxPlus{ + AllowedAddresses: []ngfAPIv1alpha1.NginxPlusAllowAddress{ + {Type: ngfAPIv1alpha1.NginxPlusAllowIPAddressType, Value: "127.0.0.3"}, + }, + }, + }, + }, + }, + }, + expNginxPlus: NginxPlus{AllowedAddresses: []string{"127.0.0.3"}}, + }, + { + msg: "NginxProxy specifies multiple allowed addresses", + g: &graph.Graph{ + NginxProxy: &graph.NginxProxy{ + Valid: true, + Source: &ngfAPIv1alpha1.NginxProxy{ + Spec: ngfAPIv1alpha1.NginxProxySpec{ + NginxPlus: &ngfAPIv1alpha1.NginxPlus{ + AllowedAddresses: []ngfAPIv1alpha1.NginxPlusAllowAddress{ + {Type: ngfAPIv1alpha1.NginxPlusAllowIPAddressType, Value: "127.0.0.3"}, + {Type: ngfAPIv1alpha1.NginxPlusAllowIPAddressType, Value: "25.0.0.3"}, + }, + }, + }, + }, + }, + }, + expNginxPlus: NginxPlus{AllowedAddresses: []string{"127.0.0.3", "25.0.0.3"}}, + }, + { + msg: "NginxProxy specifies 127.0.0.1 as allowed address", + g: &graph.Graph{ + NginxProxy: &graph.NginxProxy{ + Valid: true, + Source: &ngfAPIv1alpha1.NginxProxy{ + Spec: ngfAPIv1alpha1.NginxProxySpec{ + NginxPlus: &ngfAPIv1alpha1.NginxPlus{ + AllowedAddresses: []ngfAPIv1alpha1.NginxPlusAllowAddress{ + {Type: ngfAPIv1alpha1.NginxPlusAllowIPAddressType, Value: "127.0.0.1"}, + }, + }, + }, + }, + }, + }, + expNginxPlus: defaultNginxPlus, + }, + } + + for _, tc := range tests { + t.Run(tc.msg, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + g.Expect(buildNginxPlus(tc.g)).To(Equal(tc.expNginxPlus)) + }) + } +} diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go index 8a46dbf126..5d2eac9551 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -50,6 +50,8 @@ type Configuration struct { Telemetry Telemetry // Logging defines logging related settings for NGINX. Logging Logging + // NginxPlus specifies NGINX Plus additional settings. + NginxPlus NginxPlus // BaseHTTPConfig holds the configuration options at the http context. BaseHTTPConfig BaseHTTPConfig // Version represents the version of the generated configuration. @@ -400,6 +402,12 @@ type Logging struct { ErrorLevel string } +// NginxPlus specifies NGINX Plus additional settings. +type NginxPlus struct { + // AllowedAddresses specifies IPAddresses or CIDR blocks to the allow list for accessing the NGINX Plus API. + AllowedAddresses []string +} + // DeploymentContext contains metadata about NGF and the cluster. // This is JSON marshaled into a file created by the generator, hence the json tags. type DeploymentContext struct { diff --git a/internal/mode/static/state/graph/nginxproxy.go b/internal/mode/static/state/graph/nginxproxy.go index 01e6e6de5b..831a205120 100644 --- a/internal/mode/static/state/graph/nginxproxy.go +++ b/internal/mode/static/state/graph/nginxproxy.go @@ -133,6 +133,8 @@ func validateNginxProxy( allErrs = append(allErrs, validateRewriteClientIP(npCfg)...) + allErrs = append(allErrs, validateNginxPlus(npCfg)...) + return allErrs } @@ -216,15 +218,15 @@ func validateRewriteClientIP(npCfg *ngfAPI.NginxProxy) field.ErrorList { valuePath := trustedAddressesPath.Child("value") switch addr.Type { - case ngfAPI.CIDRAddressType: + case ngfAPI.RewriteClientIPCIDRAddressType: if err := k8svalidation.IsValidCIDR(valuePath, addr.Value); err != nil { allErrs = append(allErrs, err...) } - case ngfAPI.IPAddressType: + case ngfAPI.RewriteClientIPIPAddressType: if err := k8svalidation.IsValidIP(valuePath, addr.Value); err != nil { allErrs = append(allErrs, err...) } - case ngfAPI.HostnameAddressType: + case ngfAPI.RewriteClientIPHostnameAddressType: if errs := k8svalidation.IsDNS1123Subdomain(addr.Value); len(errs) > 0 { for _, e := range errs { allErrs = append(allErrs, field.Invalid(valuePath, addr.Value, e)) @@ -236,9 +238,9 @@ func validateRewriteClientIP(npCfg *ngfAPI.NginxProxy) field.ErrorList { field.NotSupported(trustedAddressesPath.Child("type"), addr.Type, []string{ - string(ngfAPI.CIDRAddressType), - string(ngfAPI.IPAddressType), - string(ngfAPI.HostnameAddressType), + string(ngfAPI.RewriteClientIPCIDRAddressType), + string(ngfAPI.RewriteClientIPIPAddressType), + string(ngfAPI.RewriteClientIPHostnameAddressType), }, ), ) @@ -248,3 +250,43 @@ func validateRewriteClientIP(npCfg *ngfAPI.NginxProxy) field.ErrorList { return allErrs } + +func validateNginxPlus(npCfg *ngfAPI.NginxProxy) field.ErrorList { + var allErrs field.ErrorList + spec := field.NewPath("spec") + + if npCfg.Spec.NginxPlus != nil { + nginxPlus := npCfg.Spec.NginxPlus + nginxPlusPath := spec.Child("nginxPlus") + + if nginxPlus.AllowedAddresses != nil { + for _, addr := range nginxPlus.AllowedAddresses { + valuePath := nginxPlusPath.Child("value") + + switch addr.Type { + case ngfAPI.NginxPlusAllowCIDRAddressType: + if err := k8svalidation.IsValidCIDR(valuePath, addr.Value); err != nil { + allErrs = append(allErrs, err...) + } + case ngfAPI.NginxPlusAllowIPAddressType: + if err := k8svalidation.IsValidIP(valuePath, addr.Value); err != nil { + allErrs = append(allErrs, err...) + } + default: + allErrs = append( + allErrs, + field.NotSupported(nginxPlusPath.Child("type"), + addr.Type, + []string{ + string(ngfAPI.NginxPlusAllowCIDRAddressType), + string(ngfAPI.NginxPlusAllowIPAddressType), + }, + ), + ) + } + } + } + } + + return allErrs +} diff --git a/internal/mode/static/state/graph/nginxproxy_test.go b/internal/mode/static/state/graph/nginxproxy_test.go index 8c688d9812..325a996321 100644 --- a/internal/mode/static/state/graph/nginxproxy_test.go +++ b/internal/mode/static/state/graph/nginxproxy_test.go @@ -275,17 +275,17 @@ func TestValidateNginxProxy(t *testing.T) { IPFamily: helpers.GetPointer[ngfAPI.IPFamilyType](ngfAPI.Dual), RewriteClientIP: &ngfAPI.RewriteClientIP{ SetIPRecursively: helpers.GetPointer(true), - TrustedAddresses: []ngfAPI.Address{ + TrustedAddresses: []ngfAPI.RewriteClientIPAddress{ { - Type: ngfAPI.CIDRAddressType, + Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32", }, { - Type: ngfAPI.IPAddressType, + Type: ngfAPI.RewriteClientIPIPAddressType, Value: "1.1.1.1", }, { - Type: ngfAPI.HostnameAddressType, + Type: ngfAPI.RewriteClientIPHostnameAddressType, Value: "example.com", }, }, @@ -399,25 +399,25 @@ func TestValidateRewriteClientIP(t *testing.T) { Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ SetIPRecursively: helpers.GetPointer(true), - TrustedAddresses: []ngfAPI.Address{ + TrustedAddresses: []ngfAPI.RewriteClientIPAddress{ { - Type: ngfAPI.CIDRAddressType, + Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32", }, { - Type: ngfAPI.CIDRAddressType, + Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "10.56.32.11/32", }, { - Type: ngfAPI.IPAddressType, + Type: ngfAPI.RewriteClientIPIPAddressType, Value: "1.1.1.1", }, { - Type: ngfAPI.IPAddressType, + Type: ngfAPI.RewriteClientIPIPAddressType, Value: "2001:db8:a0b:12f0::1", }, { - Type: ngfAPI.HostnameAddressType, + Type: ngfAPI.RewriteClientIPHostnameAddressType, Value: "example.com", }, }, @@ -434,13 +434,13 @@ func TestValidateRewriteClientIP(t *testing.T) { Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ SetIPRecursively: helpers.GetPointer(true), - TrustedAddresses: []ngfAPI.Address{ + TrustedAddresses: []ngfAPI.RewriteClientIPAddress{ { - Type: ngfAPI.CIDRAddressType, + Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8::/129", }, { - Type: ngfAPI.CIDRAddressType, + Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "10.0.0.1/32", }, }, @@ -459,13 +459,13 @@ func TestValidateRewriteClientIP(t *testing.T) { Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ SetIPRecursively: helpers.GetPointer(true), - TrustedAddresses: []ngfAPI.Address{ + TrustedAddresses: []ngfAPI.RewriteClientIPAddress{ { - Type: ngfAPI.IPAddressType, + Type: ngfAPI.RewriteClientIPIPAddressType, Value: "1.2.3.4.5", }, { - Type: ngfAPI.IPAddressType, + Type: ngfAPI.RewriteClientIPIPAddressType, Value: "10.0.0.1", }, }, @@ -484,13 +484,13 @@ func TestValidateRewriteClientIP(t *testing.T) { Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ SetIPRecursively: helpers.GetPointer(true), - TrustedAddresses: []ngfAPI.Address{ + TrustedAddresses: []ngfAPI.RewriteClientIPAddress{ { - Type: ngfAPI.HostnameAddressType, + Type: ngfAPI.RewriteClientIPHostnameAddressType, Value: "bad-host$%^", }, { - Type: ngfAPI.HostnameAddressType, + Type: ngfAPI.RewriteClientIPHostnameAddressType, Value: "example.com", }, }, @@ -524,28 +524,28 @@ func TestValidateRewriteClientIP(t *testing.T) { Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeProxyProtocol), - TrustedAddresses: []ngfAPI.Address{ - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, - {Type: ngfAPI.CIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + TrustedAddresses: []ngfAPI.RewriteClientIPAddress{ + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, }, }, }, @@ -560,13 +560,13 @@ func TestValidateRewriteClientIP(t *testing.T) { Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ Mode: helpers.GetPointer(ngfAPI.RewriteClientIPModeType("invalid")), - TrustedAddresses: []ngfAPI.Address{ + TrustedAddresses: []ngfAPI.RewriteClientIPAddress{ { - Type: ngfAPI.CIDRAddressType, + Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32", }, { - Type: ngfAPI.CIDRAddressType, + Type: ngfAPI.RewriteClientIPCIDRAddressType, Value: "10.0.0.1/32", }, }, @@ -599,9 +599,9 @@ func TestValidateRewriteClientIP(t *testing.T) { Spec: ngfAPI.NginxProxySpec{ RewriteClientIP: &ngfAPI.RewriteClientIP{ SetIPRecursively: helpers.GetPointer(true), - TrustedAddresses: []ngfAPI.Address{ + TrustedAddresses: []ngfAPI.RewriteClientIPAddress{ { - Type: ngfAPI.AddressType("invalid"), + Type: ngfAPI.RewriteClientIPAddressType("invalid"), Value: "2001:db8::/129", }, }, @@ -773,3 +773,107 @@ func TestValidateLogging(t *testing.T) { }) } } + +func TestValidateNginxPlus(t *testing.T) { + t.Parallel() + + tests := []struct { + np *ngfAPI.NginxProxy + name string + errorString string + expectErrCount int + }{ + { + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + NginxPlus: &ngfAPI.NginxPlus{ + AllowedAddresses: []ngfAPI.NginxPlusAllowAddress{ + {Type: ngfAPI.NginxPlusAllowIPAddressType, Value: "2001:db8:a0b:12f0::1"}, + {Type: ngfAPI.NginxPlusAllowCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.NginxPlusAllowIPAddressType, Value: "127.0.0.3"}, + {Type: ngfAPI.NginxPlusAllowCIDRAddressType, Value: "127.0.0.3/32"}, + }, + }, + }, + }, + name: "valid NginxPlus", + errorString: "", + expectErrCount: 0, + }, + { + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + NginxPlus: &ngfAPI.NginxPlus{ + AllowedAddresses: []ngfAPI.NginxPlusAllowAddress{ + {Type: ngfAPI.NginxPlusAllowCIDRAddressType, Value: "2001:db8:a0b:12f0::1/32"}, + {Type: ngfAPI.NginxPlusAllowCIDRAddressType, Value: "127.0.0.3/37"}, + }, + }, + }, + }, + name: "invalid CIDR in AllowedAddresses", + errorString: "spec.nginxPlus.value: Invalid value: \"127.0.0.3/37\": must be a valid CIDR value, " + + "(e.g. 10.9.8.0/24 or 2001:db8::/64)", + expectErrCount: 1, + }, + { + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + NginxPlus: &ngfAPI.NginxPlus{ + AllowedAddresses: []ngfAPI.NginxPlusAllowAddress{ + {Type: ngfAPI.NginxPlusAllowIPAddressType, Value: "127.0.0.3"}, + {Type: ngfAPI.NginxPlusAllowIPAddressType, Value: "127.0.0.3.5/32"}, + }, + }, + }, + }, + name: "invalid IP address in AllowedAddresses", + errorString: "spec.nginxPlus.value: Invalid value: \"127.0.0.3.5/32\": must be a valid IP address, " + + "(e.g. 10.9.8.7 or 2001:db8::ffff)", + expectErrCount: 1, + }, + { + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + NginxPlus: &ngfAPI.NginxPlus{ + AllowedAddresses: []ngfAPI.NginxPlusAllowAddress{ + {Type: ngfAPI.NginxPlusAllowAddressType("Hostname"), Value: "example.com"}, + }, + }, + }, + }, + name: "hostname type in AllowedAddresses", + errorString: "spec.nginxPlus.type: Unsupported value: \"Hostname\": supported " + + "values: \"CIDR\", \"IPAddress\"", + expectErrCount: 1, + }, + { + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + NginxPlus: &ngfAPI.NginxPlus{ + AllowedAddresses: []ngfAPI.NginxPlusAllowAddress{ + {Type: ngfAPI.NginxPlusAllowAddressType("invalid"), Value: "example.com"}, + }, + }, + }, + }, + name: "invalid type in AllowedAddresses", + errorString: "spec.nginxPlus.type: Unsupported value: \"invalid\": supported " + + "values: \"CIDR\", \"IPAddress\"", + expectErrCount: 1, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + allErrs := validateNginxPlus(test.np) + g.Expect(allErrs).To(HaveLen(test.expectErrCount)) + if len(allErrs) > 0 { + g.Expect(allErrs.ToAggregate().Error()).To(Equal(test.errorString)) + } + }) + } +} diff --git a/internal/mode/static/state/graph/route_common.go b/internal/mode/static/state/graph/route_common.go index 2db3e9e0b4..31a7c1528f 100644 --- a/internal/mode/static/state/graph/route_common.go +++ b/internal/mode/static/state/graph/route_common.go @@ -337,22 +337,96 @@ func bindRoutesToListeners( bindL7RouteToListeners(r, gw, namespaces) } - var routes []*L4Route - for _, r := range l4Routes { + routes := make([]*L7Route, 0, len(l7Routes)) + for _, r := range l7Routes { routes = append(routes, r) } + isolateL7RouteListeners(routes, gw.Listeners) + + l4RouteSlice := make([]*L4Route, 0, len(l4Routes)) + for _, r := range l4Routes { + l4RouteSlice = append(l4RouteSlice, r) + } + // Sort the slice by timestamp and name so that we process the routes in the priority order - sort.Slice(routes, func(i, j int) bool { - return ngfSort.LessClientObject(routes[i].Source, routes[j].Source) + sort.Slice(l4RouteSlice, func(i, j int) bool { + return ngfSort.LessClientObject(l4RouteSlice[i].Source, l4RouteSlice[j].Source) }) // portHostnamesMap exists to detect duplicate hostnames on the same port portHostnamesMap := make(map[string]struct{}) - for _, r := range routes { + for _, r := range l4RouteSlice { bindL4RouteToListeners(r, gw, namespaces, portHostnamesMap) } + + isolateL4RouteListeners(l4RouteSlice, gw.Listeners) +} + +// isolateL7RouteListeners ensures listener isolation for all L7Routes. +func isolateL7RouteListeners(routes []*L7Route, listeners []*Listener) { + listenerHostnameMap := make(map[string]string, len(listeners)) + for _, l := range listeners { + listenerHostnameMap[l.Name] = getHostname(l.Source.Hostname) + } + + for _, route := range routes { + isolateHostnamesForParentRefs(route.ParentRefs, listenerHostnameMap) + } +} + +// isolateL4RouteListeners ensures listener isolation for all L4Routes. +func isolateL4RouteListeners(routes []*L4Route, listeners []*Listener) { + listenerHostnameMap := make(map[string]string, len(listeners)) + for _, l := range listeners { + listenerHostnameMap[l.Name] = getHostname(l.Source.Hostname) + } + + for _, route := range routes { + isolateHostnamesForParentRefs(route.ParentRefs, listenerHostnameMap) + } +} + +// isolateHostnamesForParentRefs iterates through the parentRefs of a route to identify the list of accepted hostnames +// for each listener. If any accepted hostname belongs to another listener, +// it removes those hostnames to ensure listener isolation. +func isolateHostnamesForParentRefs(parentRef []ParentRef, listenerHostnameMap map[string]string) { + for _, ref := range parentRef { + acceptedHostnames := ref.Attachment.AcceptedHostnames + + hostnamesToRemoves := make(map[string]struct{}) + for listenerName, hostnames := range acceptedHostnames { + if len(hostnames) == 0 { + continue + } + for _, h := range hostnames { + for lName, lHostname := range listenerHostnameMap { + // skip comparison if it is a catch all listener block + if lHostname == "" { + continue + } + if h == lHostname && listenerName != lName { + hostnamesToRemoves[h] = struct{}{} + } + } + } + + isolatedHostnames := removeHostnames(hostnames, hostnamesToRemoves) + ref.Attachment.AcceptedHostnames[listenerName] = isolatedHostnames + } + } +} + +// removeHostnames removes the hostnames that are part of toRemove slice. +func removeHostnames(hostnames []string, toRemove map[string]struct{}) []string { + result := make([]string, 0, len(hostnames)) + for _, hostname := range hostnames { + if _, exists := toRemove[hostname]; !exists { + result = append(result, hostname) + } + } + return result } func validateParentRef( diff --git a/internal/mode/static/state/graph/route_common_test.go b/internal/mode/static/state/graph/route_common_test.go index 42f601e5df..32df93de4a 100644 --- a/internal/mode/static/state/graph/route_common_test.go +++ b/internal/mode/static/state/graph/route_common_test.go @@ -2175,3 +2175,551 @@ func TestTryToAttachL4RouteToListeners_NoAttachableListeners(t *testing.T) { g.Expect(cond).To(Equal(staticConds.NewRouteInvalidListener())) g.Expect(attachable).To(BeFalse()) } + +func TestIsolateL4Listeners(t *testing.T) { + t.Parallel() + gw := &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "gateway", + }, + } + + createTLSRouteWithSectionNameAndPort := func( + name string, + sectionName *gatewayv1.SectionName, + ns string, + hostnames ...gatewayv1.Hostname, + ) *v1alpha2.TLSRoute { + return &v1alpha2.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: name, + }, + Spec: v1alpha2.TLSRouteSpec{ + CommonRouteSpec: gatewayv1.CommonRouteSpec{ + ParentRefs: []gatewayv1.ParentReference{ + { + Name: gatewayv1.ObjectName(gw.Name), + SectionName: sectionName, + }, + }, + }, + Hostnames: hostnames, + }, + } + } + + routeHostnames := []gatewayv1.Hostname{"bar.com", "*.example.com", "*.foo.example.com", "abc.foo.example.com"} + tr1 := createTLSRouteWithSectionNameAndPort( + "tr1", + helpers.GetPointer[gatewayv1.SectionName]("empty-hostname"), + "test", + routeHostnames..., + ) + tr2 := createTLSRouteWithSectionNameAndPort( + "tr2", + helpers.GetPointer[gatewayv1.SectionName]("wildcard-example-com"), + "test", + routeHostnames..., + ) + tr3 := createTLSRouteWithSectionNameAndPort( + "tr3", + helpers.GetPointer[gatewayv1.SectionName]("foo-wildcard-example-com"), + "test", + routeHostnames..., + ) + tr4 := createTLSRouteWithSectionNameAndPort( + "tr4", + helpers.GetPointer[gatewayv1.SectionName]("abc-com"), + "test", + routeHostnames..., + ) + tr5 := createTLSRouteWithSectionNameAndPort( + "tr5", + helpers.GetPointer[gatewayv1.SectionName]("no-match"), + "test", + routeHostnames..., + ) + + createL4RoutewithAcceptedHostnames := func( + source *v1alpha2.TLSRoute, + acceptedHostnames map[string][]string, + hostnames []gatewayv1.Hostname, + sectionName *gatewayv1.SectionName, + ) *L4Route { + return &L4Route{ + Source: source, + Spec: L4RouteSpec{ + Hostnames: hostnames, + }, + ParentRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKey{ + Namespace: gw.Namespace, + Name: gw.Name, + }, + SectionName: sectionName, + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: acceptedHostnames, + Attached: true, + }, + }, + }, + } + } + + createListener := func(name string, hostname string) *Listener { + return &Listener{ + Name: name, + Source: gatewayv1.Listener{ + Name: gatewayv1.SectionName(name), + Hostname: (*gatewayv1.Hostname)(helpers.GetPointer(hostname)), + }, + Valid: true, + Attachable: true, + } + } + + acceptedHostnamesEmptyHostname := map[string][]string{ + "empty-hostname": { + "bar.com", "*.example.com", "*.foo.example.com", "abc.foo.example.com", + }, + } + acceptedHostnamesWildcardExample := map[string][]string{ + "wildcard-example-com": { + "*.example.com", "*.foo.example.com", "abc.foo.example.com", + }, + } + + acceptedHostnamesFooWildcardExample := map[string][]string{ + "foo-wildcard-example-com": { + "*.foo.example.com", "abc.foo.example.com", + }, + } + + acceptedHostnamesAbcCom := map[string][]string{ + "abc-com": { + "abc.foo.example.com", + }, + } + acceptedHostnamesNoMatch := map[string][]string{ + "no-match": {}, + } + + routes := []*L4Route{ + createL4RoutewithAcceptedHostnames( + tr1, acceptedHostnamesEmptyHostname, + []gatewayv1.Hostname{"bar.com"}, + helpers.GetPointer[gatewayv1.SectionName]("empty-hostname"), + ), + createL4RoutewithAcceptedHostnames( + tr2, + acceptedHostnamesWildcardExample, + []gatewayv1.Hostname{"*.example.com"}, + helpers.GetPointer[gatewayv1.SectionName]("wildcard-example-com"), + ), + createL4RoutewithAcceptedHostnames( + tr3, + acceptedHostnamesFooWildcardExample, + []gatewayv1.Hostname{"*.foo.example.com"}, + helpers.GetPointer[gatewayv1.SectionName]("foo-wildcard-example-com"), + ), + createL4RoutewithAcceptedHostnames( + tr4, + acceptedHostnamesAbcCom, + []gatewayv1.Hostname{"abc.foo.example.com"}, + helpers.GetPointer[gatewayv1.SectionName]("abc-com"), + ), + createL4RoutewithAcceptedHostnames( + tr5, + acceptedHostnamesNoMatch, + []gatewayv1.Hostname{"cafe.example.com"}, + helpers.GetPointer[gatewayv1.SectionName]("no-match"), + ), + } + + listeners := []*Listener{ + createListener("empty-hostname", ""), + createListener("wildcard-example-com", "*.example.com"), + createListener("foo-wildcard-example-com", "*.foo.example.com"), + createListener("abc-com", "abc.foo.example.com"), + createListener("no-match", "no-match.cafe.com"), + } + + expectedResult := map[string][]ParentRef{ + "tr1": { + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + SectionName: tr1.Spec.ParentRefs[0].SectionName, + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + "empty-hostname": {"bar.com"}, + }, + Attached: true, + }, + }, + }, + "tr2": { + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + SectionName: tr2.Spec.ParentRefs[0].SectionName, + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + "wildcard-example-com": {"*.example.com"}, + }, + Attached: true, + }, + }, + }, + "tr3": { + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + SectionName: tr3.Spec.ParentRefs[0].SectionName, + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + "foo-wildcard-example-com": {"*.foo.example.com"}, + }, + Attached: true, + }, + }, + }, + "tr4": { + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + SectionName: tr4.Spec.ParentRefs[0].SectionName, + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + "abc-com": {"abc.foo.example.com"}, + }, + Attached: true, + }, + }, + }, + "tr5": { + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + SectionName: tr5.Spec.ParentRefs[0].SectionName, + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + "no-match": {}, + }, + Attached: true, + }, + }, + }, + } + + g := NewWithT(t) + isolateL4RouteListeners(routes, listeners) + + result := map[string][]ParentRef{} + for _, route := range routes { + result[route.Source.GetName()] = route.ParentRefs + } + g.Expect(helpers.Diff(result, expectedResult)).To(BeEmpty()) +} + +func TestIsolateL7Listeners(t *testing.T) { + t.Parallel() + gw := &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "gateway", + }, + } + + createHTTPRouteWithSectionNameAndPort := func( + name string, + sectionName *gatewayv1.SectionName, + ns string, + hostnames ...gatewayv1.Hostname, + ) *gatewayv1.HTTPRoute { + return &gatewayv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: name, + }, + Spec: gatewayv1.HTTPRouteSpec{ + CommonRouteSpec: gatewayv1.CommonRouteSpec{ + ParentRefs: []gatewayv1.ParentReference{ + { + Name: gatewayv1.ObjectName(gw.Name), + SectionName: sectionName, + }, + }, + }, + Hostnames: hostnames, + }, + } + } + + routeHostnames := []gatewayv1.Hostname{"bar.com", "*.example.com", "*.foo.example.com", "abc.foo.example.com"} + hr1 := createHTTPRouteWithSectionNameAndPort( + "hr1", + helpers.GetPointer[gatewayv1.SectionName]("empty-hostname"), + "test", + routeHostnames..., + ) + hr2 := createHTTPRouteWithSectionNameAndPort( + "hr2", + helpers.GetPointer[gatewayv1.SectionName]("wildcard-example-com"), + "test", + routeHostnames..., + ) + hr3 := createHTTPRouteWithSectionNameAndPort( + "hr3", + helpers.GetPointer[gatewayv1.SectionName]("foo-wildcard-example-com"), + "test", + routeHostnames..., + ) + hr4 := createHTTPRouteWithSectionNameAndPort( + "hr4", + helpers.GetPointer[gatewayv1.SectionName]("abc-com"), + "test", + routeHostnames..., + ) + hr5 := createHTTPRouteWithSectionNameAndPort( + "hr5", + helpers.GetPointer[gatewayv1.SectionName]("no-match"), + "test", + routeHostnames..., // no matching hostname + ) + + createL7RoutewithAcceptedHostnames := func( + source *gatewayv1.HTTPRoute, + acceptedHostnames map[string][]string, + hostnames []gatewayv1.Hostname, + sectionName *gatewayv1.SectionName, + ) *L7Route { + return &L7Route{ + Source: source, + Spec: L7RouteSpec{ + Hostnames: hostnames, + }, + ParentRefs: []ParentRef{ + { + Idx: 0, + Gateway: client.ObjectKey{ + Namespace: gw.Namespace, + Name: gw.Name, + }, + SectionName: sectionName, + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: acceptedHostnames, + Attached: true, + }, + }, + }, + } + } + + createListener := func(name string, hostname string) *Listener { + return &Listener{ + Name: name, + Source: gatewayv1.Listener{ + Name: gatewayv1.SectionName(name), + Hostname: (*gatewayv1.Hostname)(helpers.GetPointer(hostname)), + }, + Valid: true, + Attachable: true, + } + } + + acceptedHostnamesEmptyHostname := map[string][]string{ + "empty-hostname": { + "bar.com", "*.example.com", "*.foo.example.com", "abc.foo.example.com", + }, + } + acceptedHostnamesWildcardExample := map[string][]string{ + "wildcard-example-com": { + "*.example.com", "*.foo.example.com", "abc.foo.example.com", + }, + } + + acceptedHostnamesFooWildcardExample := map[string][]string{ + "foo-wildcard-example-com": { + "*.foo.example.com", "abc.foo.example.com", + }, + } + + acceptedHostnamesAbcCom := map[string][]string{ + "abc-com": { + "abc.foo.example.com", + }, + } + acceptedHostnamesNoMatch := map[string][]string{ + "no-match": {}, + } + + routes := []*L7Route{ + createL7RoutewithAcceptedHostnames( + hr1, + acceptedHostnamesEmptyHostname, + []gatewayv1.Hostname{"bar.com"}, + helpers.GetPointer[gatewayv1.SectionName]("empty-hostname"), + ), + createL7RoutewithAcceptedHostnames( + hr2, + acceptedHostnamesWildcardExample, + []gatewayv1.Hostname{"*.example.com"}, + helpers.GetPointer[gatewayv1.SectionName]("wildcard-example-com"), + ), + createL7RoutewithAcceptedHostnames( + hr3, + acceptedHostnamesFooWildcardExample, + []gatewayv1.Hostname{"*.foo.example.com"}, + helpers.GetPointer[gatewayv1.SectionName]("foo-wildcard-example-com"), + ), + createL7RoutewithAcceptedHostnames( + hr4, + acceptedHostnamesAbcCom, + []gatewayv1.Hostname{"abc.foo.example.com"}, + helpers.GetPointer[gatewayv1.SectionName]("abc-com"), + ), + createL7RoutewithAcceptedHostnames( + hr5, + acceptedHostnamesNoMatch, + []gatewayv1.Hostname{"cafe.example.com"}, + helpers.GetPointer[gatewayv1.SectionName]("no-match"), + ), + } + + listeners := []*Listener{ + createListener("empty-hostname", ""), + createListener("wildcard-example-com", "*.example.com"), + createListener("foo-wildcard-example-com", "*.foo.example.com"), + createListener("abc-com", "abc.foo.example.com"), + createListener("no-match", "no-match.cafe.com"), + } + + expectedResult := map[string][]ParentRef{ + "hr1": { + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + SectionName: hr1.Spec.ParentRefs[0].SectionName, + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + "empty-hostname": {"bar.com"}, + }, + Attached: true, + }, + }, + }, + "hr2": { + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + SectionName: hr2.Spec.ParentRefs[0].SectionName, + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + "wildcard-example-com": {"*.example.com"}, + }, + Attached: true, + }, + }, + }, + "hr3": { + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + SectionName: hr3.Spec.ParentRefs[0].SectionName, + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + "foo-wildcard-example-com": {"*.foo.example.com"}, + }, + Attached: true, + }, + }, + }, + "hr4": { + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + SectionName: hr4.Spec.ParentRefs[0].SectionName, + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + "abc-com": {"abc.foo.example.com"}, + }, + Attached: true, + }, + }, + }, + "hr5": { + { + Idx: 0, + Gateway: client.ObjectKeyFromObject(gw), + SectionName: hr5.Spec.ParentRefs[0].SectionName, + Attachment: &ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + "no-match": {}, + }, + Attached: true, + }, + }, + }, + } + + g := NewWithT(t) + isolateL7RouteListeners(routes, listeners) + + result := map[string][]ParentRef{} + for _, route := range routes { + result[route.Source.GetName()] = route.ParentRefs + } + + g.Expect(helpers.Diff(result, expectedResult)).To(BeEmpty()) +} + +func TestRemoveHostnames(t *testing.T) { + t.Parallel() + tests := []struct { + name string + hostnames []string + removeHostnames map[string]struct{} + expectedHostnames []string + }{ + { + name: "remove multiple hostnames", + hostnames: []string{"foo.example.com", "bar.example.com", "bar.com", "*.wildcard.com"}, + removeHostnames: map[string]struct{}{ + "foo.example.com": {}, + "bar.example.com": {}, + }, + expectedHostnames: []string{"bar.com", "*.wildcard.com"}, + }, + { + name: "remove all hostnames", + hostnames: []string{"foo.example.com", "bar.example.com", "bar.com", "*.wildcard.com"}, + removeHostnames: map[string]struct{}{ + "foo.example.com": {}, + "bar.example.com": {}, + "bar.com": {}, + "*.wildcard.com": {}, + }, + expectedHostnames: []string{}, + }, + { + name: "remove no hostnames", + hostnames: []string{"foo.example.com", "bar.example.com", "bar.com", "*.wildcard.com"}, + removeHostnames: map[string]struct{}{}, + expectedHostnames: []string{"foo.example.com", "bar.example.com", "bar.com", "*.wildcard.com"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + result := removeHostnames(tt.hostnames, tt.removeHostnames) + g.Expect(result).To(Equal(tt.expectedHostnames)) + }) + } +} diff --git a/internal/mode/static/status/prepare_requests_test.go b/internal/mode/static/status/prepare_requests_test.go index 702cbe9827..5d7eb9f2ea 100644 --- a/internal/mode/static/status/prepare_requests_test.go +++ b/internal/mode/static/status/prepare_requests_test.go @@ -5,6 +5,7 @@ import ( "errors" "testing" + "github.com/go-logr/logr" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -13,7 +14,6 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - "sigs.k8s.io/controller-runtime/pkg/log/zap" v1 "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1alpha2" "sigs.k8s.io/gateway-api/apis/v1alpha3" @@ -268,7 +268,7 @@ func TestBuildHTTPRouteStatuses(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) } - updater := statusFramework.NewUpdater(k8sClient, zap.New()) + updater := statusFramework.NewUpdater(k8sClient, logr.Discard()) reqs := PrepareRouteRequests( map[graph.L4RouteKey]*graph.L4Route{}, @@ -347,7 +347,7 @@ func TestBuildGRPCRouteStatuses(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) } - updater := statusFramework.NewUpdater(k8sClient, zap.New()) + updater := statusFramework.NewUpdater(k8sClient, logr.Discard()) reqs := PrepareRouteRequests( map[graph.L4RouteKey]*graph.L4Route{}, @@ -424,7 +424,7 @@ func TestBuildTLSRouteStatuses(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) } - updater := statusFramework.NewUpdater(k8sClient, zap.New()) + updater := statusFramework.NewUpdater(k8sClient, logr.Discard()) reqs := PrepareRouteRequests( routes, @@ -528,7 +528,7 @@ func TestBuildRouteStatusesNginxErr(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) } - updater := statusFramework.NewUpdater(k8sClient, zap.New()) + updater := statusFramework.NewUpdater(k8sClient, logr.Discard()) reqs := PrepareRouteRequests( map[graph.L4RouteKey]*graph.L4Route{}, @@ -662,7 +662,7 @@ func TestBuildGatewayClassStatuses(t *testing.T) { expectedTotalReqs++ } - updater := statusFramework.NewUpdater(k8sClient, zap.New()) + updater := statusFramework.NewUpdater(k8sClient, logr.Discard()) reqs := PrepareGatewayClassRequests(test.gc, test.ignoredClasses, transitionTime) @@ -1158,7 +1158,7 @@ func TestBuildGatewayStatuses(t *testing.T) { expectedTotalReqs++ } - updater := statusFramework.NewUpdater(k8sClient, zap.New()) + updater := statusFramework.NewUpdater(k8sClient, logr.Discard()) reqs := PrepareGatewayRequests( test.gateway, @@ -1376,7 +1376,7 @@ func TestBuildBackendTLSPolicyStatuses(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) } - updater := statusFramework.NewUpdater(k8sClient, zap.New()) + updater := statusFramework.NewUpdater(k8sClient, logr.Discard()) reqs := PrepareBackendTLSPolicyRequests(test.backendTLSPolicies, transitionTime, gatewayCtlrName) @@ -1470,7 +1470,7 @@ func TestBuildNginxGatewayStatus(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) } - updater := statusFramework.NewUpdater(k8sClient, zap.New()) + updater := statusFramework.NewUpdater(k8sClient, logr.Discard()) req := PrepareNginxGatewayStatus(test.nginxGateway, transitionTime, test.cpUpdateResult) @@ -1764,7 +1764,7 @@ func TestBuildNGFPolicyStatuses(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) } - updater := statusFramework.NewUpdater(k8sClient, zap.New()) + updater := statusFramework.NewUpdater(k8sClient, logr.Discard()) reqs := PrepareNGFPolicyRequests(test.policies, transitionTime, gatewayCtlrName) @@ -1894,7 +1894,7 @@ func TestBuildSnippetsFilterStatuses(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) } - updater := statusFramework.NewUpdater(k8sClient, zap.New()) + updater := statusFramework.NewUpdater(k8sClient, logr.Discard()) reqs := PrepareSnippetsFilterRequests(test.snippetsFilters, transitionTime, gatewayCtlrName) diff --git a/internal/mode/static/telemetry/job_worker_test.go b/internal/mode/static/telemetry/job_worker_test.go index 770af9892c..8855fd5d8b 100644 --- a/internal/mode/static/telemetry/job_worker_test.go +++ b/internal/mode/static/telemetry/job_worker_test.go @@ -6,9 +6,9 @@ import ( "testing" "time" + "github.com/go-logr/logr" tel "github.com/nginx/telemetry-exporter/pkg/telemetry" . "github.com/onsi/gomega" - "sigs.k8s.io/controller-runtime/pkg/log/zap" "github.com/nginx/nginx-gateway-fabric/internal/mode/static/telemetry" "github.com/nginx/nginx-gateway-fabric/internal/mode/static/telemetry/telemetryfakes" @@ -21,7 +21,7 @@ func TestCreateTelemetryJobWorker_Succeeds(t *testing.T) { exporter := &telemetryfakes.FakeExporter{} dataCollector := &telemetryfakes.FakeDataCollector{} - worker := telemetry.CreateTelemetryJobWorker(zap.New(), exporter, dataCollector) + worker := telemetry.CreateTelemetryJobWorker(logr.Discard(), exporter, dataCollector) expData := telemetry.Data{ Data: tel.Data{ @@ -46,7 +46,7 @@ func TestCreateTelemetryJobWorker_CollectFails(t *testing.T) { exporter := &telemetryfakes.FakeExporter{} dataCollector := &telemetryfakes.FakeDataCollector{} - worker := telemetry.CreateTelemetryJobWorker(zap.New(), exporter, dataCollector) + worker := telemetry.CreateTelemetryJobWorker(logr.Discard(), exporter, dataCollector) expData := telemetry.Data{} dataCollector.CollectReturns(expData, errors.New("failed to collect cluster information")) diff --git a/site/content/reference/api.md b/site/content/reference/api.md index 77eafa4ac6..0f829e7ac7 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -364,13 +364,26 @@ NginxLogging +nginxPlus
+ + +NginxPlus + + + + +(Optional) +

NginxPlus specifies NGINX Plus additional settings.

+ + + + disableHTTP2
bool -(Optional)

DisableHTTP2 defines if http2 should be disabled for all servers. Default is false, meaning http2 will be enabled for all servers.

@@ -707,78 +720,6 @@ sigs.k8s.io/gateway-api/apis/v1alpha2.PolicyStatus -

Address - -

-

-(Appears on: -RewriteClientIP) -

-

-

Address is a struct that specifies address type and value.

-

- - - - - - - - - - - - - - - - - -
FieldDescription
-type
- - -AddressType - - -		 -

Type specifies the type of address.

-
-value
- -string - -		 -

Value specifies the address value.

-
-

AddressType -(string alias)

-

-

-(Appears on: -Address) -

-

-

AddressType specifies the type of address.

-

- - - - - - - - - - - - - - -
ValueDescription

"CIDR"

CIDRAddressType specifies that the address is a CIDR block.

-

"Hostname"

HostnameAddressType specifies that the address is a Hostname.

-

"IPAddress"

IPAddressType specifies that the address is an IP address.

-

ClientBody

@@ -1399,6 +1340,109 @@ crit, alert, and emerg messages to be logged. NginxPlus + + +

+(Appears on: +NginxProxySpec) +

+

+

NginxPlus specifies NGINX Plus additional settings. These will only be applied if NGINX Plus is being used.

+

+ + + + + + + + + + + + + +
FieldDescription
+allowedAddresses
+ + +[]NginxPlusAllowAddress + + +		 +(Optional) +

AllowedAddresses specifies IPAddresses or CIDR blocks to the allow list for accessing the NGINX Plus API.

+
+

NginxPlusAllowAddress + +

+

+(Appears on: +NginxPlus) +

+

+

NginxPlusAllowAddress specifies the address type and value for an NginxPlus allow address.

+

+ + + + + + + + + + + + + + + + + +
FieldDescription
+type
+ + +NginxPlusAllowAddressType + + +		 +

Type specifies the type of address.

+
+value
+ +string + +		 +

Value specifies the address value.

+
+

NginxPlusAllowAddressType +(string alias)

+

+

+(Appears on: +NginxPlusAllowAddress) +

+

+

NginxPlusAllowAddressType specifies the type of address.

+

+ + + + + + + + + + + + +
ValueDescription

"CIDR"

NginxPlusAllowCIDRAddressType specifies that the address is a CIDR block.

+

"IPAddress"

NginxPlusAllowIPAddressType specifies that the address is an IP address.

+

NginxProxySpec

@@ -1476,13 +1520,26 @@ NginxLogging +nginxPlus
+ + +NginxPlus + + + + +(Optional) +

NginxPlus specifies NGINX Plus additional settings.

+ + + + disableHTTP2
bool -(Optional)

DisableHTTP2 defines if http2 should be disabled for all servers. Default is false, meaning http2 will be enabled for all servers.

@@ -1598,8 +1655,8 @@ Sets NGINX directive real_ip_recursive: -[]Address + +[]RewriteClientIPAddress @@ -1618,6 +1675,78 @@ This field is required if mode is set.

+

RewriteClientIPAddress + +

+

+(Appears on: +RewriteClientIP) +

+

+

RewriteClientIPAddress specifies the address type and value for a RewriteClientIP address.

+

+ + + + + + + + + + + + + + + + + +
FieldDescription
+type
+ + +RewriteClientIPAddressType + + +		 +

Type specifies the type of address.

+
+value
+ +string + +		 +

Value specifies the address value.

+
+

RewriteClientIPAddressType +(string alias)

+

+

+(Appears on: +RewriteClientIPAddress) +

+

+

RewriteClientIPAddressType specifies the type of address.

+

+ + + + + + + + + + + + + + +
ValueDescription

"CIDR"

RewriteClientIPCIDRAddressType specifies that the address is a CIDR block.

+

"Hostname"

RewriteClientIPHostnameAddressType specifies that the address is a Hostname.

+

"IPAddress"

RewriteClientIPIPAddressType specifies that the address is an IP address.

+

RewriteClientIPModeType (string alias)

diff --git a/tests/Makefile b/tests/Makefile index 2fa1922162..c95f337d19 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -14,7 +14,7 @@ NGF_VERSION ?= edge## NGF version to be tested PULL_POLICY = Never## Pull policy for the images NGINX_CONF_DIR = internal/mode/static/nginx/conf PROVISIONER_MANIFEST = conformance/provisioner/provisioner.yaml -SUPPORTED_EXTENDED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,GatewayPort8080,HTTPRouteResponseHeaderModification,HTTPRoutePathRedirect +SUPPORTED_EXTENDED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,GatewayPort8080,HTTPRouteResponseHeaderModification,HTTPRoutePathRedirect,GatewayHTTPListenerIsolation STANDARD_CONFORMANCE_PROFILES = GATEWAY-HTTP,GATEWAY-GRPC EXPERIMENTAL_CONFORMANCE_PROFILES = GATEWAY-TLS CONFORMANCE_PROFILES = $(STANDARD_CONFORMANCE_PROFILES) # by default we use the standard conformance profiles. If experimental is enabled we override this and add the experimental profiles. diff --git a/tests/go.mod b/tests/go.mod index afcec66976..b352ee28b7 100644 --- a/tests/go.mod +++ b/tests/go.mod @@ -58,7 +58,7 @@ require ( github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/procfs v0.15.1 // indirect github.com/rs/dnscache v0.0.0-20230804202142-fc85eb664529 // indirect - github.com/spf13/pflag v1.0.5 // indirect + github.com/spf13/pflag v1.0.6 // indirect github.com/stretchr/testify v1.9.0 // indirect github.com/x448/float16 v0.8.4 // indirect golang.org/x/mod v0.22.0 // indirect diff --git a/tests/go.sum b/tests/go.sum index 7fb137e3c7..ff50d6d20e 100644 --- a/tests/go.sum +++ b/tests/go.sum @@ -113,8 +113,8 @@ github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4= github.com/rs/dnscache v0.0.0-20230804202142-fc85eb664529 h1:18kd+8ZUlt/ARXhljq+14TwAoKa61q6dX8jtwOf6DH8= github.com/rs/dnscache v0.0.0-20230804202142-fc85eb664529/go.mod h1:qe5TWALJ8/a1Lqznoc5BDHpYX/8HU60Hm2AwRmqzxqA= -github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= -github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o= +github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/streadway/quantile v0.0.0-20220407130108-4246515d968d h1:X4+kt6zM/OVO6gbJdAfJR60MGPsqCzbtXNnjoGqdfAs= github.com/streadway/quantile v0.0.0-20220407130108-4246515d968d/go.mod h1:lbP8tGiBjZ5YWIc2fzuRpTaz0b/53vT6PEs3QuAWzuU= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=