diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go new file mode 100644 index 0000000000..f6365b735e --- /dev/null +++ b/apis/v1alpha1/nginxproxy_types.go @@ -0,0 +1,68 @@ +package v1alpha1 + +import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + +// +kubebuilder:object:root=true +// +kubebuilder:storageversion +// +kubebuilder:subresource:status + +// NginxProxy represents the dynamic configuration for an NGINX Gateway Fabric data plane. +type NginxProxy struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + // NginxProxySpec defines the desired state of the NginxProxy. + Spec NginxProxySpec `json:"spec"` + + // NginxProxyStatus defines the state of the NginxProxy. + Status NginxProxyStatus `json:"status,omitempty"` +} + +// +kubebuilder:object:root=true + +// NginxProxyList contains a list of NginxProxies. +type NginxProxyList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []NginxProxy `json:"items"` +} + +// NginxProxySpec defines the desired state of the NginxProxy. +type NginxProxySpec struct{} + +// NginxProxyStatus defines the state of the NginxProxy. +type NginxProxyStatus struct { + // +optional + // +listType=map + // +listMapKey=type + // +kubebuilder:validation:MaxItems=8 + Conditions []metav1.Condition `json:"conditions,omitempty"` +} + +// NginxProxyConditionType is a type of condition associated with an +// NginxProxy. This type should be used with the NginxProxyStatus.Conditions field. +type NginxProxyConditionType string + +// NginxProxyConditionReason defines the set of reasons that explain why a +// particular NginxProxy condition type has been raised. +type NginxProxyConditionReason string + +const ( + // NginxProxyConditionAccepted is a condition that is true when the NginxProxy + // configuration is syntactically and semantically valid. + NginxProxyConditionAccepted NginxProxyConditionType = "Accepted" + + // NginxProxyReasonAccepted is a reason that is used with the "Accepted" condition when the condition is True. + NginxProxyReasonAccepted NginxProxyConditionReason = "Accepted" + + // NginxProxyConditionProgrammed is a condition that is true when the NginxProxy has resulted in + // successful nginx configuration. + NginxProxyConditionProgrammed NginxProxyConditionType = "Programmed" + + // NginxProxyReasonProgrammed is a reason that is used with the "Programmed" condition when the condition is True. + NginxProxyReasonProgrammed NginxProxyConditionReason = "Programmed" + + // NginxProxyReasonInvalid is a reason that is used with the "Accepted" or "Programmed" condition when + // an error occurs with validation or reloading nginx. + NginxProxyReasonInvalid NginxProxyConditionReason = "Invalid" +) diff --git a/apis/v1alpha1/register.go b/apis/v1alpha1/register.go index f3f36d27da..7ace4109bf 100644 --- a/apis/v1alpha1/register.go +++ b/apis/v1alpha1/register.go @@ -34,6 +34,8 @@ func addKnownTypes(scheme *runtime.Scheme) error { scheme.AddKnownTypes(SchemeGroupVersion, &NginxGateway{}, &NginxGatewayList{}, + &NginxProxy{}, + &NginxProxyList{}, ) // AddToGroupVersion allows the serialization of client types like ListOptions. metav1.AddToGroupVersion(scheme, SchemeGroupVersion) diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index fc8962cf32..767c6e5ead 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -129,3 +129,99 @@ func (in *NginxGatewayStatus) DeepCopy() *NginxGatewayStatus { 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 + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + out.Spec = in.Spec + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxProxy. +func (in *NginxProxy) DeepCopy() *NginxProxy { + if in == nil { + return nil + } + out := new(NginxProxy) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *NginxProxy) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NginxProxyList) DeepCopyInto(out *NginxProxyList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]NginxProxy, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxProxyList. +func (in *NginxProxyList) DeepCopy() *NginxProxyList { + if in == nil { + return nil + } + out := new(NginxProxyList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *NginxProxyList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NginxProxySpec) DeepCopyInto(out *NginxProxySpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxProxySpec. +func (in *NginxProxySpec) DeepCopy() *NginxProxySpec { + if in == nil { + return nil + } + out := new(NginxProxySpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NginxProxyStatus) DeepCopyInto(out *NginxProxyStatus) { + *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]v1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxProxyStatus. +func (in *NginxProxyStatus) DeepCopy() *NginxProxyStatus { + if in == nil { + return nil + } + out := new(NginxProxyStatus) + in.DeepCopyInto(out) + return out +} diff --git a/deploy/helm-chart/README.md b/deploy/helm-chart/README.md index cfc13c4294..cfba342a55 100644 --- a/deploy/helm-chart/README.md +++ b/deploy/helm-chart/README.md @@ -184,7 +184,7 @@ To uninstall/delete the release `my-release`: ```shell helm uninstall my-release -n nginx-gateway kubectl delete ns nginx-gateway -kubectl delete crd nginxgateways.gateway.nginx.org +kubectl delete crd nginxgateways.gateway.nginx.org nginxproxies.gateway.nginx.org ``` These commands remove all the Kubernetes components associated with the release and deletes the release. @@ -225,6 +225,7 @@ The following tables lists the configurable parameters of the NGINX Gateway Fabr | `nginx.image.repository` | The repository for the NGINX image. | ghcr.io/nginxinc/nginx-gateway-fabric/nginx | | `nginx.image.tag` | The tag for the NGINX image. | edge | | `nginx.image.pullPolicy` | The `imagePullPolicy` for the NGINX image. | Always | +| `nginx.config` | The configuration for the data plane that is contained in the NginxProxy resource. | [See nginx.config section](values.yaml) | | `nginx.lifecycle` | The `lifecycle` of the nginx container. | {} | | `nginx.extraVolumeMounts` | Extra `volumeMounts` for the nginx container. | {} | | `terminationGracePeriodSeconds` | The termination grace period of the NGINX Gateway Fabric pod. | 30 | diff --git a/deploy/helm-chart/crds/gateway.nginx.org_nginxproxies.yaml b/deploy/helm-chart/crds/gateway.nginx.org_nginxproxies.yaml new file mode 100644 index 0000000000..9d94a75a5c --- /dev/null +++ b/deploy/helm-chart/crds/gateway.nginx.org_nginxproxies.yaml @@ -0,0 +1,120 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.13.0 + name: nginxproxies.gateway.nginx.org +spec: + group: gateway.nginx.org + names: + kind: NginxProxy + listKind: NginxProxyList + plural: nginxproxies + singular: nginxproxy + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: NginxProxy represents the dynamic configuration for an NGINX + Gateway Fabric data plane. + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: NginxProxySpec defines the desired state of the NginxProxy. + type: object + status: + description: NginxProxyStatus defines the state of the NginxProxy. + properties: + conditions: + items: + description: "Condition contains details for one aspect of the current + state of this API Resource. --- This struct is intended for direct + use as an array at the field path .status.conditions. For example, + \n type FooStatus struct{ // Represents the observations of a + foo's current state. // Known .status.conditions.type are: \"Available\", + \"Progressing\", and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge + // +listType=map // +listMapKey=type Conditions []metav1.Condition + `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\" + protobuf:\"bytes,1,rep,name=conditions\"` \n // other fields }" + properties: + lastTransitionTime: + description: lastTransitionTime is the last time the condition + transitioned from one status to another. This should be when + the underlying condition changed. If that is not known, then + using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: message is a human readable message indicating + details about the transition. This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: observedGeneration represents the .metadata.generation + that the condition was set based upon. For instance, if .metadata.generation + is currently 12, but the .status.conditions[x].observedGeneration + is 9, the condition is out of date with respect to the current + state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: reason contains a programmatic identifier indicating + the reason for the condition's last transition. Producers + of specific condition types may define expected values and + meanings for this field, and whether the values are considered + a guaranteed API. The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + --- Many .condition.type values are consistent across resources + like Available, but because arbitrary conditions can be useful + (see .node.status.conditions), the ability to deconflict is + important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + maxItems: 8 + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + type: object + required: + - spec + type: object + served: true + storage: true + subresources: + status: {} diff --git a/deploy/helm-chart/templates/_helpers.tpl b/deploy/helm-chart/templates/_helpers.tpl index f94eeb379f..13d78128ae 100644 --- a/deploy/helm-chart/templates/_helpers.tpl +++ b/deploy/helm-chart/templates/_helpers.tpl @@ -31,6 +31,14 @@ Create control plane config name. {{- printf "%s-config" $name | trunc 63 | trimSuffix "-" }} {{- end }} +{{/* +Create data plane config name. +*/}} +{{- define "nginx-gateway.proxy-config-name" -}} +{{- $name := default .Release.Name .Values.nameOverride }} +{{- printf "%s-proxy-config" $name | trunc 63 | trimSuffix "-" }} +{{- end }} + {{/* Create chart name and version as used by the chart label. */}} diff --git a/deploy/helm-chart/templates/gatewayclass.yaml b/deploy/helm-chart/templates/gatewayclass.yaml index b1288ba38b..0bded71bb7 100644 --- a/deploy/helm-chart/templates/gatewayclass.yaml +++ b/deploy/helm-chart/templates/gatewayclass.yaml @@ -6,3 +6,8 @@ metadata: {{- include "nginx-gateway.labels" . | nindent 4 }} spec: controllerName: {{ .Values.nginxGateway.gatewayControllerName }} + parametersRef: + group: gateway.nginx.org + kind: NginxProxy + name: {{ include "nginx-gateway.proxy-config-name" . }} + namespace: {{ .Release.Namespace }} diff --git a/deploy/helm-chart/templates/nginxproxy.yaml b/deploy/helm-chart/templates/nginxproxy.yaml new file mode 100644 index 0000000000..5f051ab692 --- /dev/null +++ b/deploy/helm-chart/templates/nginxproxy.yaml @@ -0,0 +1,9 @@ +apiVersion: gateway.nginx.org/v1alpha1 +kind: NginxProxy +metadata: + name: {{ include "nginx-gateway.proxy-config-name" . }} + namespace: {{ .Release.Namespace }} + labels: + {{- include "nginx-gateway.labels" . | nindent 4 }} +spec: + {{- toYaml .Values.nginx.config | nindent 2 }} diff --git a/deploy/helm-chart/templates/rbac.yaml b/deploy/helm-chart/templates/rbac.yaml index 405c34525e..ee0b85e074 100644 --- a/deploy/helm-chart/templates/rbac.yaml +++ b/deploy/helm-chart/templates/rbac.yaml @@ -60,6 +60,7 @@ rules: - gateway.nginx.org resources: - nginxgateways + - nginxproxies verbs: - get - list @@ -68,6 +69,7 @@ rules: - gateway.nginx.org resources: - nginxgateways/status + - nginxproxies/status verbs: - update {{- if .Values.nginxGateway.leaderElection.enable }} diff --git a/deploy/helm-chart/values.yaml b/deploy/helm-chart/values.yaml index 32fdb5db68..ca80bdf430 100644 --- a/deploy/helm-chart/values.yaml +++ b/deploy/helm-chart/values.yaml @@ -58,6 +58,9 @@ nginx: tag: edge pullPolicy: Always + ## The configuration for the data plane that is contained in the NginxProxy resource. + config: {} + ## The lifecycle of the nginx container. lifecycle: {} diff --git a/deploy/manifests/crds/gateway.nginx.org_nginxproxies.yaml b/deploy/manifests/crds/gateway.nginx.org_nginxproxies.yaml new file mode 100644 index 0000000000..9d94a75a5c --- /dev/null +++ b/deploy/manifests/crds/gateway.nginx.org_nginxproxies.yaml @@ -0,0 +1,120 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.13.0 + name: nginxproxies.gateway.nginx.org +spec: + group: gateway.nginx.org + names: + kind: NginxProxy + listKind: NginxProxyList + plural: nginxproxies + singular: nginxproxy + scope: Namespaced + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: NginxProxy represents the dynamic configuration for an NGINX + Gateway Fabric data plane. + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: NginxProxySpec defines the desired state of the NginxProxy. + type: object + status: + description: NginxProxyStatus defines the state of the NginxProxy. + properties: + conditions: + items: + description: "Condition contains details for one aspect of the current + state of this API Resource. --- This struct is intended for direct + use as an array at the field path .status.conditions. For example, + \n type FooStatus struct{ // Represents the observations of a + foo's current state. // Known .status.conditions.type are: \"Available\", + \"Progressing\", and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge + // +listType=map // +listMapKey=type Conditions []metav1.Condition + `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\" + protobuf:\"bytes,1,rep,name=conditions\"` \n // other fields }" + properties: + lastTransitionTime: + description: lastTransitionTime is the last time the condition + transitioned from one status to another. This should be when + the underlying condition changed. If that is not known, then + using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: message is a human readable message indicating + details about the transition. This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: observedGeneration represents the .metadata.generation + that the condition was set based upon. For instance, if .metadata.generation + is currently 12, but the .status.conditions[x].observedGeneration + is 9, the condition is out of date with respect to the current + state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: reason contains a programmatic identifier indicating + the reason for the condition's last transition. Producers + of specific condition types may define expected values and + meanings for this field, and whether the values are considered + a guaranteed API. The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + --- Many .condition.type values are consistent across resources + like Available, but because arbitrary conditions can be useful + (see .node.status.conditions), the ability to deconflict is + important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + maxItems: 8 + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + type: object + required: + - spec + type: object + served: true + storage: true + subresources: + status: {} diff --git a/deploy/manifests/nginx-gateway.yaml b/deploy/manifests/nginx-gateway.yaml index 045c81c679..99a549e3e5 100644 --- a/deploy/manifests/nginx-gateway.yaml +++ b/deploy/manifests/nginx-gateway.yaml @@ -71,6 +71,7 @@ rules: - gateway.nginx.org resources: - nginxgateways + - nginxproxies verbs: - get - list @@ -79,6 +80,7 @@ rules: - gateway.nginx.org resources: - nginxgateways/status + - nginxproxies/status verbs: - update - apiGroups: @@ -244,6 +246,11 @@ metadata: app.kubernetes.io/version: "edge" spec: controllerName: gateway.nginx.org/nginx-gateway-controller + parametersRef: + group: gateway.nginx.org + kind: NginxProxy + name: nginx-gateway-proxy-config + namespace: nginx-gateway --- # Source: nginx-gateway-fabric/templates/nginxgateway.yaml apiVersion: gateway.nginx.org/v1alpha1 @@ -258,3 +265,16 @@ metadata: spec: logging: level: info +--- +# Source: nginx-gateway-fabric/templates/nginxproxy.yaml +apiVersion: gateway.nginx.org/v1alpha1 +kind: NginxProxy +metadata: + name: nginx-gateway-proxy-config + namespace: nginx-gateway + labels: + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/version: "edge" +spec: + {} diff --git a/docs/gateway-api-compatibility.md b/docs/gateway-api-compatibility.md index 32eb1a7703..e6e0440918 100644 --- a/docs/gateway-api-compatibility.md +++ b/docs/gateway-api-compatibility.md @@ -54,7 +54,7 @@ Fields: - `spec` - `controllerName` - supported. - - `parametersRef` - not supported. + - `parametersRef` - NginxProxy resource supported. - `description` - supported. - `status` - `conditions` - supported (Condition/Status/Reason): diff --git a/internal/framework/status/setters.go b/internal/framework/status/setters.go index 40effc14c9..eab8118575 100644 --- a/internal/framework/status/setters.go +++ b/internal/framework/status/setters.go @@ -35,6 +35,27 @@ func newNginxGatewayStatusSetter(clock Clock, status NginxGatewayStatus) func(cl } } +func newNginxProxyStatusSetter(clock Clock, status NginxProxyStatus) func(client.Object) bool { + return func(object client.Object) bool { + ng := object.(*ngfAPI.NginxProxy) + conds := convertConditions( + status.Conditions, + status.ObservedGeneration, + clock.Now(), + ) + + if conditionsEqual(ng.Status.Conditions, conds) { + return false + } + + ng.Status = ngfAPI.NginxProxyStatus{ + Conditions: conds, + } + + return true + } +} + func newGatewayClassStatusSetter(clock Clock, gcs GatewayClassStatus) func(client.Object) bool { return func(object client.Object) bool { gc := object.(*v1beta1.GatewayClass) diff --git a/internal/framework/status/setters_test.go b/internal/framework/status/setters_test.go index f379e369ee..55f9e303cf 100644 --- a/internal/framework/status/setters_test.go +++ b/internal/framework/status/setters_test.go @@ -63,6 +63,56 @@ func TestNewNginxGatewayStatusSetter(t *testing.T) { } } +func TestNewNginxProxyStatusSetter(t *testing.T) { + tests := []struct { + name string + status ngfAPI.NginxProxyStatus + newStatus NginxProxyStatus + expStatusSet bool + }{ + { + name: "NginxProxyStatus has no status", + expStatusSet: true, + newStatus: NginxProxyStatus{ + Conditions: []conditions.Condition{{Message: "new condition"}}, + }, + }, + { + name: "NginxProxyStatus has old status", + expStatusSet: true, + newStatus: NginxProxyStatus{ + Conditions: []conditions.Condition{{Message: "new condition"}}, + }, + status: ngfAPI.NginxProxyStatus{ + Conditions: []v1.Condition{{Message: "old condition"}}, + }, + }, + { + name: "NginxProxyStatus has same status", + expStatusSet: false, + newStatus: NginxProxyStatus{ + Conditions: []conditions.Condition{{Message: "same condition"}}, + }, + status: ngfAPI.NginxProxyStatus{ + Conditions: []v1.Condition{{Message: "same condition"}}, + }, + }, + } + + clock := &RealClock{} + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + setter := newNginxProxyStatusSetter(clock, test.newStatus) + + statusSet := setter(&ngfAPI.NginxProxy{Status: test.status}) + g.Expect(statusSet).To(Equal(test.expStatusSet)) + }) + } +} + func TestNewGatewayClassStatusSetter(t *testing.T) { tests := []struct { name string diff --git a/internal/framework/status/statuses.go b/internal/framework/status/statuses.go index 9af0108aa7..545c391a4a 100644 --- a/internal/framework/status/statuses.go +++ b/internal/framework/status/statuses.go @@ -39,6 +39,20 @@ func (n *NginxGatewayStatus) APIGroup() string { return ngfAPI.GroupName } +// NginxProxyStatus holds status-related information about the NginxProxy resource. +type NginxProxyStatus struct { + // NsName is the NamespacedName of the NginxProxy resource. + NsName types.NamespacedName + // Conditions is the list of conditions for this NginxProxy. + Conditions []conditions.Condition + // ObservedGeneration is the generation of the resource that was processed. + ObservedGeneration int64 +} + +func (n *NginxProxyStatus) APIGroup() string { + return ngfAPI.GroupName +} + // ListenerStatuses holds the statuses of listeners where the key is the name of a listener in the Gateway resource. type ListenerStatuses map[string]ListenerStatus diff --git a/internal/framework/status/updater.go b/internal/framework/status/updater.go index 16797ea347..28a8f18eb4 100644 --- a/internal/framework/status/updater.go +++ b/internal/framework/status/updater.go @@ -90,6 +90,7 @@ type UpdaterImpl struct { // a leader change. type lastStatuses struct { nginxGateway *NginxGatewayStatus + nginxProxy *NginxProxyStatus gatewayAPI GatewayAPIStatuses } @@ -104,6 +105,7 @@ func (upd *UpdaterImpl) Enable(ctx context.Context) { upd.cfg.Logger.Info("Writing last statuses") upd.updateGatewayAPI(ctx, upd.lastStatuses.gatewayAPI) + upd.updateNginxProxy(ctx, upd.lastStatuses.nginxProxy) upd.updateNginxGateway(ctx, upd.lastStatuses.nginxGateway) } @@ -134,6 +136,8 @@ func (upd *UpdaterImpl) Update(ctx context.Context, status Status) { switch s := status.(type) { case *NginxGatewayStatus: upd.updateNginxGateway(ctx, s) + case *NginxProxyStatus: + upd.updateNginxProxy(ctx, s) case GatewayAPIStatuses: upd.updateGatewayAPI(ctx, s) default: @@ -145,13 +149,13 @@ func (upd *UpdaterImpl) updateNginxGateway(ctx context.Context, status *NginxGat upd.lastStatuses.nginxGateway = status if !upd.isLeader { - upd.cfg.Logger.Info("Skipping updating Nginx Gateway status because not leader") + upd.cfg.Logger.Info("Skipping updating NginxGateway status because not leader") return } - upd.cfg.Logger.Info("Updating Nginx Gateway status") - if status != nil { + upd.cfg.Logger.Info("Updating NginxGateway status") + upd.writeStatuses( ctx, status.NsName, @@ -161,6 +165,26 @@ func (upd *UpdaterImpl) updateNginxGateway(ctx context.Context, status *NginxGat } } +func (upd *UpdaterImpl) updateNginxProxy(ctx context.Context, status *NginxProxyStatus) { + upd.lastStatuses.nginxProxy = status + + if !upd.isLeader { + upd.cfg.Logger.Info("Skipping updating NginxProxy status because not leader") + return + } + + if status != nil { + upd.cfg.Logger.Info("Updating NginxProxy status") + + upd.writeStatuses( + ctx, + status.NsName, + &ngfAPI.NginxProxy{}, + newNginxProxyStatusSetter(upd.cfg.Clock, *status), + ) + } +} + func (upd *UpdaterImpl) updateGatewayAPI(ctx context.Context, statuses GatewayAPIStatuses) { upd.lastStatuses.gatewayAPI = statuses diff --git a/internal/framework/status/updater_test.go b/internal/framework/status/updater_test.go index 786ce10c26..af256b0de9 100644 --- a/internal/framework/status/updater_test.go +++ b/internal/framework/status/updater_test.go @@ -51,6 +51,7 @@ var _ = Describe("Updater", func() { &v1beta1.Gateway{}, &v1beta1.HTTPRoute{}, &ngfAPI.NginxGateway{}, + &ngfAPI.NginxProxy{}, ). Build() @@ -73,6 +74,7 @@ var _ = Describe("Updater", func() { gw, ignoredGw *v1beta1.Gateway hr *v1beta1.HTTPRoute ng *ngfAPI.NginxGateway + np *ngfAPI.NginxProxy addr = v1beta1.GatewayStatusAddress{ Type: helpers.GetPointer(v1beta1.IPAddressType), Value: "1.2.3.4", @@ -131,6 +133,17 @@ var _ = Describe("Updater", func() { } } + createNPStatus = func(gen int64) *status.NginxProxyStatus { + return &status.NginxProxyStatus{ + NsName: types.NamespacedName{ + Namespace: "nginx-gateway", + Name: "nginx-gateway-proxy-config", + }, + ObservedGeneration: gen, + Conditions: status.CreateTestConditions("Test"), + } + } + createExpectedGCWithGeneration = func(generation int64) *v1beta1.GatewayClass { return &v1beta1.GatewayClass{ ObjectMeta: metav1.ObjectMeta{ @@ -247,6 +260,22 @@ var _ = Describe("Updater", func() { }, } } + + createExpectedNPWithGeneration = func(gen int64) *ngfAPI.NginxProxy { + return &ngfAPI.NginxProxy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "nginx-gateway", + Name: "nginx-gateway-proxy-config", + }, + TypeMeta: metav1.TypeMeta{ + Kind: "NginxProxy", + APIVersion: "gateway.nginx.org/v1alpha1", + }, + Status: ngfAPI.NginxProxyStatus{ + Conditions: status.CreateExpectedAPIConditions("Test", gen, fakeClockTime), + }, + } + } ) BeforeAll(func() { @@ -308,6 +337,16 @@ var _ = Describe("Updater", func() { APIVersion: "gateway.nginx.org/v1alpha1", }, } + np = &ngfAPI.NginxProxy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "nginx-gateway", + Name: "nginx-gateway-proxy-config", + }, + TypeMeta: metav1.TypeMeta{ + Kind: "NginxProxy", + APIVersion: "gateway.nginx.org/v1alpha1", + }, + } }) It("should create resources in the API server", func() { @@ -316,6 +355,7 @@ var _ = Describe("Updater", func() { Expect(client.Create(context.Background(), ignoredGw)).Should(Succeed()) Expect(client.Create(context.Background(), hr)).Should(Succeed()) Expect(client.Create(context.Background(), ng)).Should(Succeed()) + Expect(client.Create(context.Background(), np)).Should(Succeed()) }) It("should update gateway API statuses", func() { @@ -397,6 +437,26 @@ var _ = Describe("Updater", func() { Expect(helpers.Diff(expectedNG, latestNG)).To(BeEmpty()) }) + It("should update nginx proxy status", func() { + updater.Update(context.Background(), createNPStatus(1)) + }) + + It("should have the updated status of NginxProxy in the API server", func() { + latestNP := &ngfAPI.NginxProxy{} + expectedNP := createExpectedNPWithGeneration(1) + + err := client.Get( + context.Background(), + types.NamespacedName{Namespace: "nginx-gateway", Name: "nginx-gateway-proxy-config"}, + latestNP, + ) + Expect(err).ToNot(HaveOccurred()) + + expectedNP.ResourceVersion = latestNP.ResourceVersion + + Expect(helpers.Diff(expectedNP, latestNP)).To(BeEmpty()) + }) + When("the Gateway Service is updated with a new address", func() { AfterEach(func() { // reset the IP for the remaining tests diff --git a/internal/mode/static/build_statuses.go b/internal/mode/static/build_statuses.go index 2a009f5580..4c763eddaa 100644 --- a/internal/mode/static/build_statuses.go +++ b/internal/mode/static/build_statuses.go @@ -5,6 +5,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" @@ -189,3 +190,27 @@ func buildGatewayStatus( ObservedGeneration: gateway.Source.Generation, } } + +func buildNginxProxyStatus(np *ngfAPI.NginxProxy, nginxReloadRes nginxReloadResult) *status.NginxProxyStatus { + if np == nil { + return nil + } + + conds := []conditions.Condition{ + staticConds.NewNginxProxyAccepted(), + } + + if nginxReloadRes.error != nil { + conds = append(conds, staticConds.NewNginxProxyNotProgrammed(staticConds.NginxProxyMessageFailedNginxReload)) + } else { + conds = append(conds, staticConds.NewNginxProxyProgrammed()) + } + + npStatus := &status.NginxProxyStatus{ + NsName: client.ObjectKeyFromObject(np), + Conditions: conds, + ObservedGeneration: np.Generation, + } + + return npStatus +} diff --git a/internal/mode/static/build_statuses_test.go b/internal/mode/static/build_statuses_test.go index 74010237be..356d20e34b 100644 --- a/internal/mode/static/build_statuses_test.go +++ b/internal/mode/static/build_statuses_test.go @@ -10,6 +10,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status" @@ -593,3 +594,59 @@ func TestBuildGatewayStatuses(t *testing.T) { }) } } + +func TestBuildNginxProxyStatus(t *testing.T) { + np := &ngfAPI.NginxProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "np", + Namespace: "test", + Generation: 1, + }, + } + + tests := []struct { + nginxReloadRes nginxReloadResult + np *ngfAPI.NginxProxy + expected *status.NginxProxyStatus + name string + }{ + { + name: "nil NginxProxy", + expected: nil, + }, + { + name: "valid", + np: np, + expected: &status.NginxProxyStatus{ + NsName: types.NamespacedName{Namespace: "test", Name: "np"}, + ObservedGeneration: 1, + Conditions: []conditions.Condition{ + staticConds.NewNginxProxyAccepted(), + staticConds.NewNginxProxyProgrammed(), + }, + }, + }, + { + name: "invalid", + nginxReloadRes: nginxReloadResult{error: errors.New("test error")}, + np: np, + expected: &status.NginxProxyStatus{ + NsName: types.NamespacedName{Namespace: "test", Name: "np"}, + ObservedGeneration: 1, + Conditions: []conditions.Condition{ + staticConds.NewNginxProxyAccepted(), + staticConds.NewNginxProxyNotProgrammed(staticConds.NginxProxyMessageFailedNginxReload), + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + + result := buildNginxProxyStatus(test.np, test.nginxReloadRes) + g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) + }) + } +} diff --git a/internal/mode/static/handler.go b/internal/mode/static/handler.go index 5ac9812525..8dca924e02 100644 --- a/internal/mode/static/handler.go +++ b/internal/mode/static/handler.go @@ -131,6 +131,7 @@ func (h *eventHandlerImpl) HandleEventBatch(ctx context.Context, logger logr.Log } h.cfg.statusUpdater.Update(ctx, buildGatewayAPIStatuses(graph, gwAddresses, nginxReloadRes)) + h.cfg.statusUpdater.Update(ctx, buildNginxProxyStatus(graph.NginxProxy, nginxReloadRes)) } func (h *eventHandlerImpl) handleEvent(ctx context.Context, logger logr.Logger, event interface{}) { diff --git a/internal/mode/static/handler_test.go b/internal/mode/static/handler_test.go index d5f3b5377c..ce3a46521d 100644 --- a/internal/mode/static/handler_test.go +++ b/internal/mode/static/handler_test.go @@ -58,7 +58,8 @@ var _ = Describe("eventHandler", func() { Expect(fakeNginxRuntimeMgr.ReloadCallCount()).Should(Equal(1)) - Expect(fakeStatusUpdater.UpdateCallCount()).Should(Equal(1)) + // One call for Gateway API resource statuses, one call for NginxProxy status + Expect(fakeStatusUpdater.UpdateCallCount()).Should(Equal(2)) } BeforeEach(func() { diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 3eca635143..de5f8efed0 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -106,7 +106,7 @@ func StartManager(cfg config.Config) error { processor := state.NewChangeProcessorImpl(state.ChangeProcessorConfig{ GatewayCtlrName: cfg.GatewayCtlrName, GatewayClassName: cfg.GatewayClassName, - RelationshipCapturer: relationship.NewCapturerImpl(), + RelationshipCapturer: relationship.NewCapturerImpl(cfg.GatewayClassName), Logger: cfg.Logger.WithName("changeProcessor"), Validators: validation.Validators{ HTTPFieldsValidator: ngxvalidation.HTTPValidator{}, @@ -300,6 +300,12 @@ func registerControllers( { objectType: &gatewayv1beta1.ReferenceGrant{}, }, + { + objectType: &ngfAPI.NginxProxy{}, + options: []controller.Option{ + controller.WithK8sPredicate(k8spredicate.GenerationChangedPredicate{}), + }, + }, } if cfg.ConfigName != "" { @@ -349,6 +355,7 @@ func prepareFirstEventBatchPreparerArgs( &discoveryV1.EndpointSliceList{}, &gatewayv1beta1.HTTPRouteList{}, &gatewayv1beta1.ReferenceGrantList{}, + &ngfAPI.NginxProxyList{}, } if gwNsName == nil { diff --git a/internal/mode/static/manager_test.go b/internal/mode/static/manager_test.go index 4bd34230ed..044f9b3c7d 100644 --- a/internal/mode/static/manager_test.go +++ b/internal/mode/static/manager_test.go @@ -12,6 +12,7 @@ import ( metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config" ) @@ -38,6 +39,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { &gatewayv1beta1.HTTPRouteList{}, &gatewayv1beta1.GatewayList{}, &gatewayv1beta1.ReferenceGrantList{}, + &ngfAPI.NginxProxyList{}, }, }, { @@ -57,6 +59,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) { &discoveryV1.EndpointSliceList{}, &gatewayv1beta1.HTTPRouteList{}, &gatewayv1beta1.ReferenceGrantList{}, + &ngfAPI.NginxProxyList{}, }, }, } diff --git a/internal/mode/static/state/change_processor.go b/internal/mode/static/state/change_processor.go index 368556a424..d0a14e1a9a 100644 --- a/internal/mode/static/state/change_processor.go +++ b/internal/mode/static/state/change_processor.go @@ -14,9 +14,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/gateway-api/apis/v1beta1" - gwapivalidation "sigs.k8s.io/gateway-api/apis/v1beta1/validation" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/relationship" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation" @@ -94,6 +94,7 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl { Namespaces: make(map[types.NamespacedName]*apiv1.Namespace), ReferenceGrants: make(map[types.NamespacedName]*v1beta1.ReferenceGrant), Secrets: make(map[types.NamespacedName]*apiv1.Secret), + NginxProxies: make(map[types.NamespacedName]*ngfAPI.NginxProxy), } extractGVK := func(obj client.Object) schema.GroupVersionKind { @@ -158,6 +159,11 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl { store: newObjectStoreMapAdapter(clusterStore.Secrets), trackUpsertDelete: false, }, + { + gvk: extractGVK(&ngfAPI.NginxProxy{}), + store: newObjectStoreMapAdapter(clusterStore.NginxProxies), + trackUpsertDelete: false, + }, }, ) diff --git a/internal/mode/static/state/change_processor_test.go b/internal/mode/static/state/change_processor_test.go index 1a3fee8bb4..875413e86e 100644 --- a/internal/mode/static/state/change_processor_test.go +++ b/internal/mode/static/state/change_processor_test.go @@ -16,6 +16,7 @@ import ( "sigs.k8s.io/gateway-api/apis/v1alpha2" "sigs.k8s.io/gateway-api/apis/v1beta1" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller/index" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" @@ -184,6 +185,7 @@ func createScheme() *runtime.Scheme { utilruntime.Must(v1beta1.AddToScheme(scheme)) utilruntime.Must(apiv1.AddToScheme(scheme)) utilruntime.Must(discoveryV1.AddToScheme(scheme)) + utilruntime.Must(ngfAPI.AddToScheme(scheme)) return scheme } @@ -259,7 +261,7 @@ var _ = Describe("ChangeProcessor", func() { processor = state.NewChangeProcessorImpl(state.ChangeProcessorConfig{ GatewayCtlrName: controllerName, GatewayClassName: gcName, - RelationshipCapturer: relationship.NewCapturerImpl(), + RelationshipCapturer: relationship.NewCapturerImpl(gcName), Logger: zap.New(), Validators: createAlwaysValidValidators(), Scheme: createScheme(), @@ -1235,6 +1237,40 @@ var _ = Describe("ChangeProcessor", func() { }) }) }) + Describe("NginxProxy resource changes", Ordered, func() { + paramGC := gc.DeepCopy() + paramGC.Spec.ParametersRef = &v1beta1.ParametersReference{ + Group: ngfAPI.GroupName, + Kind: v1beta1.Kind("NginxProxy"), + Name: "np", + Namespace: helpers.GetPointer(v1beta1.Namespace("test")), + } + + np := &ngfAPI.NginxProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "np", + Namespace: "test", + }, + } + It("handles upserts for an NginxProxy", func() { + processor.CaptureUpsertChange(np) + processor.CaptureUpsertChange(paramGC) + + changed, graph := processor.Process() + Expect(changed).To(BeTrue()) + Expect(graph.NginxProxy).To(Equal(np)) + + // TODO(sberman): Once some fields actually exist + // for this resource, verify that an update occurs. + }) + It("handles deletes for an NginxProxy", func() { + processor.CaptureDeleteChange(np, client.ObjectKeyFromObject(np)) + + changed, graph := processor.Process() + Expect(changed).To(BeTrue()) + Expect(graph.NginxProxy).To(BeNil()) + }) + }) }) Describe("Ensuring non-changing changes don't override previously changing changes", func() { @@ -1614,7 +1650,7 @@ var _ = Describe("ChangeProcessor", func() { processor = state.NewChangeProcessorImpl(state.ChangeProcessorConfig{ GatewayCtlrName: controllerName, GatewayClassName: gcName, - RelationshipCapturer: relationship.NewCapturerImpl(), + RelationshipCapturer: relationship.NewCapturerImpl(gcName), Logger: zap.New(), Validators: createAlwaysValidValidators(), EventRecorder: fakeEventRecorder, @@ -1787,7 +1823,7 @@ var _ = Describe("ChangeProcessor", func() { processor = state.NewChangeProcessorImpl(state.ChangeProcessorConfig{ GatewayCtlrName: controllerName, GatewayClassName: gcName, - RelationshipCapturer: relationship.NewCapturerImpl(), + RelationshipCapturer: relationship.NewCapturerImpl(gcName), Logger: zap.New(), Validators: createAlwaysValidValidators(), EventRecorder: fakeEventRecorder, diff --git a/internal/mode/static/state/conditions/conditions.go b/internal/mode/static/state/conditions/conditions.go index 2154a3197e..c0e7d554a6 100644 --- a/internal/mode/static/state/conditions/conditions.go +++ b/internal/mode/static/state/conditions/conditions.go @@ -63,6 +63,11 @@ const ( // FIXME(bjee19): Update to Gateway sig v1 version when released. // https://github.com/nginxinc/nginx-gateway-fabric/issues/1168 RouteConditionPartiallyInvalid v1beta1.RouteConditionType = "PartiallyInvalid" + + // NginxProxyMessageFailedNginxReload is a message used when nginx fails to reload. + NginxProxyMessageFailedNginxReload = "There was a failure to reload nginx with the configuration. " + + "The issue could be due to this resource, Gateway resource, or Route resource. Please see the nginx " + + "container logs for any possible configuration issues" ) // DeduplicateConditions removes duplicate conditions based on the condition type. @@ -583,3 +588,34 @@ func NewNginxGatewayInvalid(msg string) conditions.Condition { Message: msg, } } + +// NewNginxProxyAccepted returns a Condition that indicates that the NginxProxy config is accepted. +func NewNginxProxyAccepted() conditions.Condition { + return conditions.Condition{ + Type: string(ngfAPI.NginxProxyConditionAccepted), + Status: metav1.ConditionTrue, + Reason: string(ngfAPI.NginxProxyReasonAccepted), + Message: "NginxProxy is accepted", + } +} + +// NewNginxProxyProgrammed returns a Condition that indicates that the NginxProxy config is programmed. +func NewNginxProxyProgrammed() conditions.Condition { + return conditions.Condition{ + Type: string(ngfAPI.NginxProxyConditionProgrammed), + Status: metav1.ConditionTrue, + Reason: string(ngfAPI.NginxProxyReasonProgrammed), + Message: "NginxProxy is programmed", + } +} + +// NewNginxProxyNotProgrammed returns a Condition that indicates that the NginxProxy config is not +// programmed due to an error to reload nginx. +func NewNginxProxyNotProgrammed(msg string) conditions.Condition { + return conditions.Condition{ + Type: string(ngfAPI.NginxProxyConditionProgrammed), + Status: metav1.ConditionFalse, + Reason: string(ngfAPI.NginxProxyReasonInvalid), + Message: msg, + } +} diff --git a/internal/mode/static/state/graph/gatewayclass.go b/internal/mode/static/state/graph/gatewayclass.go index 819f7a4860..110cce62f6 100644 --- a/internal/mode/static/state/graph/gatewayclass.go +++ b/internal/mode/static/state/graph/gatewayclass.go @@ -6,6 +6,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" ) @@ -56,14 +57,14 @@ func processGatewayClasses( return processedGwClasses, gcExists } -func buildGatewayClass(gc *v1beta1.GatewayClass) *GatewayClass { +func buildGatewayClass(gc *v1beta1.GatewayClass, npCfg *ngfAPI.NginxProxy) *GatewayClass { if gc == nil { return nil } var conds []conditions.Condition - valErr := validateGatewayClass(gc) + valErr := validateGatewayClass(gc, npCfg) if valErr != nil { conds = append(conds, staticConds.NewGatewayClassInvalidParameters(valErr.Error())) } @@ -75,11 +76,25 @@ func buildGatewayClass(gc *v1beta1.GatewayClass) *GatewayClass { } } -func validateGatewayClass(gc *v1beta1.GatewayClass) error { +func validateGatewayClass(gc *v1beta1.GatewayClass, npCfg *ngfAPI.NginxProxy) error { if gc.Spec.ParametersRef != nil { path := field.NewPath("spec").Child("parametersRef") - return field.Forbidden(path, "parametersRef is not supported") + if _, ok := supportedParamKinds[string(gc.Spec.ParametersRef.Kind)]; !ok { + return field.NotSupported(path.Child("kind"), string(gc.Spec.ParametersRef.Kind), []string{"NginxProxy"}) + } + + if gc.Spec.ParametersRef.Namespace == nil { + return field.Required(path.Child("namespace"), "parametersRef.namespace must be specified for NginxProxy") + } + + if npCfg == nil { + return field.NotFound(path.Child("name"), gc.Spec.ParametersRef.Name) + } } return nil } + +var supportedParamKinds = map[string]struct{}{ + "NginxProxy": {}, +} diff --git a/internal/mode/static/state/graph/gatewayclass_test.go b/internal/mode/static/state/graph/gatewayclass_test.go index b5e899b00b..64b7d9b253 100644 --- a/internal/mode/static/state/graph/gatewayclass_test.go +++ b/internal/mode/static/state/graph/gatewayclass_test.go @@ -9,6 +9,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions" @@ -121,14 +122,36 @@ func TestProcessGatewayClasses(t *testing.T) { func TestBuildGatewayClass(t *testing.T) { validGC := &v1beta1.GatewayClass{} - invalidGC := &v1beta1.GatewayClass{ + gcWithParams := &v1beta1.GatewayClass{ Spec: v1beta1.GatewayClassSpec{ - ParametersRef: &v1beta1.ParametersReference{}, + ParametersRef: &v1beta1.ParametersReference{ + Kind: v1beta1.Kind("NginxProxy"), + Namespace: helpers.GetPointer(v1beta1.Namespace("test")), + Name: "does-not-exist", + }, + }, + } + + gcWithInvalidKind := &v1beta1.GatewayClass{ + Spec: v1beta1.GatewayClassSpec{ + ParametersRef: &v1beta1.ParametersReference{ + Kind: v1beta1.Kind("Invalid"), + Namespace: helpers.GetPointer(v1beta1.Namespace("test")), + }, + }, + } + + gcWithNoNamespace := &v1beta1.GatewayClass{ + Spec: v1beta1.GatewayClassSpec{ + ParametersRef: &v1beta1.ParametersReference{ + Kind: v1beta1.Kind("NginxProxy"), + }, }, } tests := []struct { gc *v1beta1.GatewayClass + np *ngfAPI.NginxProxy expected *GatewayClass name string }{ @@ -146,17 +169,66 @@ func TestBuildGatewayClass(t *testing.T) { name: "no gatewayclass", }, { - gc: invalidGC, + gc: gcWithParams, + np: &ngfAPI.NginxProxy{ + TypeMeta: metav1.TypeMeta{ + Kind: "NginxProxy", + }, + }, + expected: &GatewayClass{ + Source: gcWithParams, + Valid: true, + }, + name: "valid gatewayclass with paramsRef", + }, + { + gc: gcWithInvalidKind, + np: &ngfAPI.NginxProxy{ + TypeMeta: metav1.TypeMeta{ + Kind: "NginxProxy", + }, + }, + expected: &GatewayClass{ + Source: gcWithInvalidKind, + Valid: false, + Conditions: []conditions.Condition{ + staticConds.NewGatewayClassInvalidParameters( + "spec.parametersRef.kind: Unsupported value: \"Invalid\": supported values: \"NginxProxy\"", + ), + }, + }, + name: "invalid gatewayclass with unsupported paramsRef Kind", + }, + { + gc: gcWithParams, + expected: &GatewayClass{ + Source: gcWithParams, + Valid: false, + Conditions: []conditions.Condition{ + staticConds.NewGatewayClassInvalidParameters( + "spec.parametersRef.name: Not found: \"does-not-exist\"", + ), + }, + }, + name: "invalid gatewayclass with paramsRef resource that doesn't exist", + }, + { + gc: gcWithNoNamespace, + np: &ngfAPI.NginxProxy{ + TypeMeta: metav1.TypeMeta{ + Kind: "NginxProxy", + }, + }, expected: &GatewayClass{ - Source: invalidGC, + Source: gcWithNoNamespace, Valid: false, Conditions: []conditions.Condition{ staticConds.NewGatewayClassInvalidParameters( - "spec.parametersRef: Forbidden: parametersRef is not supported", + "spec.parametersRef.namespace: Required value: parametersRef.namespace must be specified for NginxProxy", ), }, }, - name: "invalid gatewayclass", + name: "invalid gatewayclass without required paramsRef Namespace", }, } @@ -164,7 +236,7 @@ func TestBuildGatewayClass(t *testing.T) { t.Run(test.name, func(t *testing.T) { g := NewWithT(t) - result := buildGatewayClass(test.gc) + result := buildGatewayClass(test.gc, test.np) g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty()) }) } diff --git a/internal/mode/static/state/graph/graph.go b/internal/mode/static/state/graph/graph.go index f5b0102ead..0ded21e92e 100644 --- a/internal/mode/static/state/graph/graph.go +++ b/internal/mode/static/state/graph/graph.go @@ -6,6 +6,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation" ) @@ -18,6 +19,7 @@ type ClusterState struct { Namespaces map[types.NamespacedName]*v1.Namespace ReferenceGrants map[types.NamespacedName]*v1beta1.ReferenceGrant Secrets map[types.NamespacedName]*v1.Secret + NginxProxies map[types.NamespacedName]*ngfAPI.NginxProxy } // Graph is a Graph-like representation of Gateway API resources. @@ -41,6 +43,8 @@ type Graph struct { // in the cluster. We need such entries so that we can query the Graph to determine if a Secret is referenced // by the Gateway, including the case when the Secret is newly created. ReferencedSecrets map[types.NamespacedName]*Secret + // NginxProxy holds the NginxProxy config for the GatewayClass. + NginxProxy *ngfAPI.NginxProxy } // ProtectedPorts are the ports that may not be configured by a listener with a descriptive name of each port. @@ -75,7 +79,9 @@ func BuildGraph( // configured GatewayClass does not reference this controller return &Graph{} } - gc := buildGatewayClass(processedGwClasses.Winner) + + npCfg := getNginxProxyConfig(state.NginxProxies, processedGwClasses.Winner) + gc := buildGatewayClass(processedGwClasses.Winner, npCfg) secretResolver := newSecretResolver(state.Secrets) @@ -95,6 +101,7 @@ func BuildGraph( IgnoredGatewayClasses: processedGwClasses.Ignored, IgnoredGateways: processedGws.Ignored, ReferencedSecrets: secretResolver.getResolvedSecrets(), + NginxProxy: npCfg, } return g diff --git a/internal/mode/static/state/graph/nginxproxy.go b/internal/mode/static/state/graph/nginxproxy.go new file mode 100644 index 0000000000..d589ec62d0 --- /dev/null +++ b/internal/mode/static/state/graph/nginxproxy.go @@ -0,0 +1,24 @@ +package graph + +import ( + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/gateway-api/apis/v1beta1" + + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" +) + +func getNginxProxyConfig( + nps map[types.NamespacedName]*ngfAPI.NginxProxy, + gc *v1beta1.GatewayClass, +) *ngfAPI.NginxProxy { + if gc != nil { + ref := gc.Spec.ParametersRef + if ref != nil && ref.Namespace != nil && + ref.Group == ngfAPI.GroupName && ref.Kind == v1beta1.Kind("NginxProxy") { + nsName := types.NamespacedName{Name: ref.Name, Namespace: string(*ref.Namespace)} + return nps[nsName] + } + } + + return nil +} diff --git a/internal/mode/static/state/relationship/capturer.go b/internal/mode/static/state/relationship/capturer.go index bd76ccd5e3..0a56efdf04 100644 --- a/internal/mode/static/state/relationship/capturer.go +++ b/internal/mode/static/state/relationship/capturer.go @@ -9,6 +9,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller/index" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" ) @@ -58,17 +59,20 @@ type CapturerImpl struct { serviceRefCount serviceRefCountMap gatewayLabelSelectors gatewayLabelSelectorsMap namespaces namespaces + paramsRef *v1beta1.ParametersReference endpointSliceOwners map[types.NamespacedName]types.NamespacedName + gcName string } // NewCapturerImpl creates a new instance of CapturerImpl. -func NewCapturerImpl() *CapturerImpl { +func NewCapturerImpl(gcName string) *CapturerImpl { return &CapturerImpl{ routesToServices: make(routeToServicesMap), serviceRefCount: make(serviceRefCountMap), gatewayLabelSelectors: make(gatewayLabelSelectorsMap), namespaces: make(namespaces), endpointSliceOwners: make(map[types.NamespacedName]types.NamespacedName), + gcName: gcName, } } @@ -86,38 +90,7 @@ func (c *CapturerImpl) Capture(obj client.Object) { } } case *v1beta1.Gateway: - var selectors []labels.Selector - for _, listener := range o.Spec.Listeners { - if selector := graph.GetAllowedRouteLabelSelector(listener); selector != nil { - convertedSelector, err := metav1.LabelSelectorAsSelector(selector) - if err == nil { - selectors = append(selectors, convertedSelector) - } - } - } - - gatewayName := client.ObjectKeyFromObject(o) - if len(selectors) > 0 { - c.gatewayLabelSelectors[gatewayName] = selectors - for ns, cfg := range c.namespaces { - var gatewayMatches bool - for _, selector := range selectors { - if selector.Matches(labels.Set(cfg.labelMap)) { - gatewayMatches = true - cfg.gateways[gatewayName] = struct{}{} - break - } - } - if !gatewayMatches { - // if gateway was previously referenced by this namespace, clean it up - delete(cfg.gateways, gatewayName) - } - c.namespaces[ns] = cfg - } - } else if _, exists := c.gatewayLabelSelectors[gatewayName]; exists { - // label selectors existed previously for this gateway, so clean up any references to them - c.removeGatewayLabelSelector(gatewayName) - } + c.upsertForGateway(o) case *v1.Namespace: nsLabels := o.GetLabels() gateways := c.matchingGateways(nsLabels) @@ -126,6 +99,10 @@ func (c *CapturerImpl) Capture(obj client.Object) { gateways: gateways, } c.namespaces[client.ObjectKeyFromObject(o)] = nsCfg + case *v1beta1.GatewayClass: + if o.Spec.ParametersRef != nil && o.Name == c.gcName { + c.paramsRef = o.Spec.ParametersRef + } } } @@ -140,6 +117,10 @@ func (c *CapturerImpl) Remove(resourceType client.Object, nsname types.Namespace c.removeGatewayLabelSelector(nsname) case *v1.Namespace: delete(c.namespaces, nsname) + case *v1beta1.GatewayClass: + if nsname.Name == c.gcName { + c.paramsRef = nil + } } } @@ -154,6 +135,14 @@ func (c *CapturerImpl) Exists(resourceType client.Object, nsname types.Namespace case *v1.Namespace: cfg, exists := c.namespaces[nsname] return exists && cfg.match() + case *ngfAPI.NginxProxy: + if c.paramsRef != nil { + return c.paramsRef.Namespace != nil && + c.paramsRef.Group == ngfAPI.GroupName && + c.paramsRef.Kind == v1beta1.Kind("NginxProxy") && + c.paramsRef.Name == nsname.Name && + string(*c.paramsRef.Namespace) == nsname.Namespace + } } return false @@ -226,6 +215,41 @@ func getBackendServiceNamesFromRoute(hr *v1beta1.HTTPRoute) map[types.Namespaced return svcNames } +func (c *CapturerImpl) upsertForGateway(gw *v1beta1.Gateway) { + var selectors []labels.Selector + for _, listener := range gw.Spec.Listeners { + if selector := graph.GetAllowedRouteLabelSelector(listener); selector != nil { + convertedSelector, err := metav1.LabelSelectorAsSelector(selector) + if err == nil { + selectors = append(selectors, convertedSelector) + } + } + } + + gatewayName := client.ObjectKeyFromObject(gw) + if len(selectors) > 0 { + c.gatewayLabelSelectors[gatewayName] = selectors + for ns, cfg := range c.namespaces { + var gatewayMatches bool + for _, selector := range selectors { + if selector.Matches(labels.Set(cfg.labelMap)) { + gatewayMatches = true + cfg.gateways[gatewayName] = struct{}{} + break + } + } + if !gatewayMatches { + // if gateway was previously referenced by this namespace, clean it up + delete(cfg.gateways, gatewayName) + } + c.namespaces[ns] = cfg + } + } else if _, exists := c.gatewayLabelSelectors[gatewayName]; exists { + // label selectors existed previously for this gateway, so clean up any references to them + c.removeGatewayLabelSelector(gatewayName) + } +} + // matchingGateways looks through all existing label selectors defined by listeners in a gateway, // and if any matches are found, returns a map of those gateways func (c *CapturerImpl) matchingGateways(labelMap map[string]string) map[types.NamespacedName]struct{} { diff --git a/internal/mode/static/state/relationship/capturer_test.go b/internal/mode/static/state/relationship/capturer_test.go index 8f9321ef4b..53762ccc1d 100644 --- a/internal/mode/static/state/relationship/capturer_test.go +++ b/internal/mode/static/state/relationship/capturer_test.go @@ -10,6 +10,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/gateway-api/apis/v1beta1" + ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller/index" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/relationship" @@ -78,7 +79,7 @@ var _ = Describe("Capturer", func() { Describe("Capture service relationships for routes", func() { BeforeEach(OncePerOrdered, func() { - capturer = relationship.NewCapturerImpl() + capturer = relationship.NewCapturerImpl("") }) assertServiceExists := func(svcName types.NamespacedName, exists bool, refCount int) { @@ -252,7 +253,7 @@ var _ = Describe("Capturer", func() { ) BeforeEach(OncePerOrdered, func() { - capturer = relationship.NewCapturerImpl() + capturer = relationship.NewCapturerImpl("") }) Describe("Normal cases", Ordered, func() { @@ -320,7 +321,7 @@ var _ = Describe("Capturer", func() { var nsNoLabels, ns *v1.Namespace BeforeEach(func() { - capturer = relationship.NewCapturerImpl() + capturer = relationship.NewCapturerImpl("") gw = &v1beta1.Gateway{ ObjectMeta: metav1.ObjectMeta{ Name: "gw", @@ -441,9 +442,95 @@ var _ = Describe("Capturer", func() { }) }) }) + Describe("Capture gatewayclass relationships for nginxproxies", Ordered, func() { + BeforeEach(func() { + capturer = relationship.NewCapturerImpl("gc") + }) + + referencedNP := &ngfAPI.NginxProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + Namespace: "test", + }, + } + + nonReferencedNP := &ngfAPI.NginxProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "invalid", + Namespace: "test", + }, + } + + When("a gatewayclass is created without paramRefs", func() { + It("does not report a relationship", func() { + gc := &v1beta1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gc", + }, + } + capturer.Capture(gc) + + Expect(capturer.Exists(referencedNP, client.ObjectKeyFromObject(referencedNP))).To(BeFalse()) + }) + }) + When("a gatewayclass is created with paramRefs but wrong name", func() { + It("does not report a relationship", func() { + gc := &v1beta1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gc2", + }, + Spec: v1beta1.GatewayClassSpec{ + ParametersRef: &v1beta1.ParametersReference{ + Group: ngfAPI.GroupName, + Kind: v1beta1.Kind("NginxProxy"), + Name: referencedNP.Name, + Namespace: helpers.GetPointer(v1beta1.Namespace(referencedNP.Namespace)), + }, + }, + } + capturer.Capture(gc) + + Expect(capturer.Exists(referencedNP, client.ObjectKeyFromObject(referencedNP))).To(BeFalse()) + }) + }) + When("a gatewayclass is created with paramRefs", func() { + gc := &v1beta1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gc", + }, + Spec: v1beta1.GatewayClassSpec{ + ParametersRef: &v1beta1.ParametersReference{ + Group: ngfAPI.GroupName, + Kind: v1beta1.Kind("NginxProxy"), + Name: referencedNP.Name, + Namespace: helpers.GetPointer(v1beta1.Namespace(referencedNP.Namespace)), + }, + }, + } + + When("an NginxProxy is created that isn't referenced", func() { + It("does not report a relationship", func() { + capturer.Capture(gc) + Expect(capturer.Exists(nonReferencedNP, client.ObjectKeyFromObject(nonReferencedNP))).To(BeFalse()) + }) + }) + When("an NginxProxy is created that is referenced", func() { + It("reports a relationship", func() { + capturer.Capture(gc) + Expect(capturer.Exists(referencedNP, client.ObjectKeyFromObject(referencedNP))).To(BeTrue()) + }) + }) + When("a gatewayclass is deleted", func() { + It("does not report a relationship", func() { + capturer.Remove(gc, client.ObjectKeyFromObject(gc)) + Expect(capturer.Exists(referencedNP, client.ObjectKeyFromObject(referencedNP))).To(BeFalse()) + }) + }) + }) + }) Describe("Edge cases", func() { BeforeEach(func() { - capturer = relationship.NewCapturerImpl() + capturer = relationship.NewCapturerImpl("") }) It("Capture does not panic when passed an unsupported resource type", func() { Expect(func() { diff --git a/internal/mode/static/state/relationship/relationships_test.go b/internal/mode/static/state/relationship/relationships_test.go index 838ac52172..d246ce93a1 100644 --- a/internal/mode/static/state/relationship/relationships_test.go +++ b/internal/mode/static/state/relationship/relationships_test.go @@ -146,7 +146,7 @@ func TestCapturerImpl_DecrementRouteCount(t *testing.T) { }, } - capturer := NewCapturerImpl() + capturer := NewCapturerImpl("") // gcName is not used in this test svc := types.NamespacedName{Namespace: "test", Name: "svc"} for _, tc := range testcases {