Create -enable-oidc command line argument for OIDC policy (#2580)

* Add new cli flag

* Remove preview from description

* update flag

* Fix linting

* Update documentation

* Remove all preview references from codebase.

* Revert the whitespace changes to the Helm README

* Update deployments/helm-chart/README.md

Co-authored-by: Luca Comellini <luca.com@gmail.com>

* Apply suggestions from code review

Co-authored-by: Michael Pleshakov <pleshakov@users.noreply.github.com>

Co-authored-by: Ciara Stacke <c.stacke@f5.com>
Co-authored-by: Ciara Stacke <18287516+ciarams87@users.noreply.github.com>
Co-authored-by: Luca Comellini <luca.com@gmail.com>
Co-authored-by: Michael Pleshakov <pleshakov@users.noreply.github.com>

cmd/nginx-ingress/main.go

@@ -163,7 +163,10 @@ var (
 		"Enable custom resources")
 
 	enablePreviewPolicies = flag.Bool("enable-preview-policies", false,
-		"Enable preview policies")
+		"Enable preview policies. This flag is deprecated. To enable OIDC Policies please use -enable-oidc instead.")
+
+	enableOIDC = flag.Bool("enable-oidc", false,
+		"Enable OIDC Policies.")
 
 	enableSnippets = flag.Bool("enable-snippets", false,
 		"Enable custom NGINX configuration snippets in Ingress, VirtualServer, VirtualServerRoute and TransportServer resources.")
@@ -250,6 +253,11 @@ func main() {
 		glog.Fatal("enable-tls-passthrough flag requires -enable-custom-resources")
 	}
 
+	if *enablePreviewPolicies {
+		glog.Warning("enable-preview-policies is universally deprecated. To enable OIDC Policies please use -enable-oidc instead.")
+	}
+	*enableOIDC = *enablePreviewPolicies || *enableOIDC
+
 	if *appProtect && !*nginxPlus {
 		glog.Fatal("NGINX App Protect support is for NGINX Plus only")
 	}
@@ -580,7 +588,7 @@ func main() {
 		MainAppProtectLoadModule:       *appProtect,
 		MainAppProtectDosLoadModule:    *appProtectDos,
 		EnableLatencyMetrics:           *enableLatencyMetrics,
-		EnablePreviewPolicies:          *enablePreviewPolicies,
+		EnableOIDC:                     *enableOIDC,
 		SSLRejectHandshake:             sslRejectHandshake,
 		EnableCertManager:              *enableCertManager,
 	}
@@ -690,7 +698,7 @@ func main() {
 		ConfigMaps:                   *nginxConfigMaps,
 		GlobalConfiguration:          *globalConfiguration,
 		AreCustomResourcesEnabled:    *enableCustomResources,
-		EnablePreviewPolicies:        *enablePreviewPolicies,
+		EnableOIDC:                   *enableOIDC,
 		MetricsCollector:             controllerCollector,
 		GlobalConfigurationValidator: globalConfigurationValidator,
 		TransportServerValidator:     transportServerValidator,

deployments/common/crds/k8s.nginx.org_policies.yaml

@@ -99,7 +99,7 @@ spec:
                     token:
                       type: string
                 oidc:
-                  description: 'OIDC defines an Open ID Connect policy. policy status: preview'
+                  description: OIDC defines an Open ID Connect policy.
                   type: object
                   properties:
                     authEndpoint:

deployments/helm-chart/README.md

@@ -182,7 +182,8 @@ Parameter | Description | Default
 `controller.setAsDefaultIngress` | New Ingresses without an `"ingressClassName"` field specified will be assigned the class specified in `controller.ingressClass`. | false
 `controller.watchNamespace` | Namespace to watch for Ingress resources. By default the Ingress controller watches all namespaces. | ""
 `controller.enableCustomResources` | Enable the custom resources. | true
-`controller.enablePreviewPolicies` | Enable preview policies. | false
+`controller.enablePreviewPolicies` | Enable preview policies. This parameter is deprecated. To enable OIDC Policies please use `controller.enableOIDC` instead. | false
+`controller.enableOIDC` | Enable OIDC policies. | false
 `controller.enableTLSPassthrough` | Enable TLS Passthrough on port 443. Requires `controller.enableCustomResources`. | false
 `controller.enableCertManager` | Enable x509 automated certificate management for VirtualServer resources using cert-manager (cert-manager.io). Requires `controller.enableCustomResources`. | false
 `controller.globalConfiguration.create` | Creates the GlobalConfiguration custom resource. Requires `controller.enableCustomResources`. | false
@@ -216,7 +217,7 @@ Parameter | Description | Default
 `controller.serviceAccount.imagePullSecretName` | The name of the secret containing docker registry credentials. Secret must exist in the same namespace as the helm release. | ""
 `controller.reportIngressStatus.enable` | Updates the address field in the status of Ingress resources with an external address of the Ingress controller. You must also specify the source of the external address either through an external service via `controller.reportIngressStatus.externalService`, `controller.reportIngressStatus.ingressLink` or the `external-status-address` entry in the ConfigMap via `controller.config.entries`. **Note:** `controller.config.entries.external-status-address` takes precedence over the others. | true
 `controller.reportIngressStatus.externalService` | Specifies the name of the service with the type LoadBalancer through which the Ingress controller is exposed externally. The external address of the service is used when reporting the status of Ingress, VirtualServer and VirtualServerRoute resources. `controller.reportIngressStatus.enable` must be set to `true`. The default is autogenerated and enabled when `controller.service.create` is set to `true` and `controller.service.type` is set to `LoadBalancer`. | Autogenerated
-`controller.reportIngressStatus.ingressLink` |  Specifies the name of the IngressLink resource, which exposes the Ingress Controller pods via a BIG-IP system. The IP of the BIG-IP system is used when reporting the status of Ingress, VirtualServer and VirtualServerRoute resources. `controller.reportIngressStatus.enable` must be set to `true`. | ""
+`controller.reportIngressStatus.ingressLink` | Specifies the name of the IngressLink resource, which exposes the Ingress Controller pods via a BIG-IP system. The IP of the BIG-IP system is used when reporting the status of Ingress, VirtualServer and VirtualServerRoute resources. `controller.reportIngressStatus.enable` must be set to `true`. | ""
 `controller.reportIngressStatus.enableLeaderElection` | Enable Leader election to avoid multiple replicas of the controller reporting the status of Ingress resources. `controller.reportIngressStatus.enable` must be set to `true`. | true
 `controller.reportIngressStatus.leaderElectionLockName` | Specifies the name of the ConfigMap, within the same namespace as the controller, used as the lock for leader election. controller.reportIngressStatus.enableLeaderElection must be set to true. | Autogenerated
 `controller.reportIngressStatus.annotations` | The annotations of the leader election configmap. | {}
@@ -230,7 +231,7 @@ Parameter | Description | Default
 `controller.appprotectdos.memory` | RAM memory size to consume in MB. | 50% of free RAM in the container or 80MB, the smaller
 `controller.readyStatus.enable` | Enables the readiness endpoint `"/nginx-ready"`. The endpoint returns a success code when NGINX has loaded all the config after the startup. This also configures a readiness probe for the Ingress Controller pods that uses the readiness endpoint. | true
 `controller.readyStatus.port` | The HTTP port for the readiness endpoint. | 8081
-`controller.enableLatencyMetrics` |  Enable collection of latency metrics for upstreams. Requires `prometheus.create`. | false
+`controller.enableLatencyMetrics` | Enable collection of latency metrics for upstreams. Requires `prometheus.create`. | false
 `rbac.create` | Configures RBAC. | true
 `prometheus.create` | Expose NGINX or NGINX Plus metrics in the Prometheus format. | false
 `prometheus.port` | Configures the port to scrape the metrics. | 9113

deployments/helm-chart/crds/k8s.nginx.org_policies.yaml

@@ -99,7 +99,7 @@ spec:
                     token:
                       type: string
                 oidc:
-                  description: 'OIDC defines an Open ID Connect policy. policy status: preview'
+                  description: OIDC defines an Open ID Connect policy.
                   type: object
                   properties:
                     authEndpoint:

deployments/helm-chart/templates/controller-daemonset.yaml

@@ -182,6 +182,7 @@ spec:
           - -enable-tls-passthrough={{ .Values.controller.enableTLSPassthrough }}
           - -enable-preview-policies={{ .Values.controller.enablePreviewPolicies }}
           - -enable-cert-manager={{ .Values.controller.enableCertManager }}
+          - -enable-oidc={{ .Values.controller.enableOIDC }}
 {{- if .Values.controller.globalConfiguration.create }}
           - -global-configuration=$(POD_NAMESPACE)/{{ include "nginx-ingress.name" . }}
 {{- end }}

deployments/helm-chart/templates/controller-deployment.yaml

@@ -180,6 +180,7 @@ spec:
           - -enable-tls-passthrough={{ .Values.controller.enableTLSPassthrough }}
           - -enable-preview-policies={{ .Values.controller.enablePreviewPolicies }}
           - -enable-cert-manager={{ .Values.controller.enableCertManager }}
+          - -enable-oidc={{ .Values.controller.enableOIDC }}
 {{- if .Values.controller.globalConfiguration.create }}
           - -global-configuration=$(POD_NAMESPACE)/{{ include "nginx-ingress.name" . }}
 {{- end }}

deployments/helm-chart/values.yaml

@@ -162,9 +162,12 @@ controller:
   ## Enable the custom resources.
   enableCustomResources: true
 
-  ## Enable preview policies.
+  ## Enable preview policies. This parameter is deprecated. To enable OIDC Policies please use controller.enableOIDC instead.
   enablePreviewPolicies: false
 
+  ## Enable OIDC policies.
+  enableOIDC: false
+
   ## Enable TLS Passthrough on port 443. Requires controller.enableCustomResources.
   enableTLSPassthrough: false
 

docs/content/configuration/global-configuration/command-line-arguments.md

@@ -59,7 +59,15 @@ Default `true`.
 
 ### -enable-preview-policies
 
-Enables preview policies.
+Enables preview policies. This flag is deprecated. To enable OIDC Policies please[-enable-oidc](#cmdoption-enable-oidc) instead.
+
+Default `false`.
+ 
+<a name="cmdoption-enable-oidc"></a>
+
+### -enable-oidc
+
+Enables OIDC policies.
 
 Default `false`.
 &nbsp;

docs/content/configuration/policy-resource.md

@@ -276,7 +276,7 @@ In this example the Ingress Controller will use the configuration from the first
 
 ### OIDC
 
-> **Feature Status**: OIDC is available as a preview feature[^1]: We might introduce some backward-incompatible changes to the resource definition. The feature is disabled by default. To enable it, set the [enable-preview-policies](/nginx-ingress-controller/configuration/global-configuration/command-line-arguments/#cmdoption-enable-preview-policies) command-line argument of the Ingress Controller.
+> **Feature Status**: This feature is disabled by default. To enable it, set the [enable-oidc](/nginx-ingress-controller/configuration/global-configuration/command-line-arguments/#cmdoption-enable-oidc) command-line argument of the Ingress Controller.
 
 The OIDC policy configures NGINX Plus as a relying party for OpenID Connect authentication.
 
@@ -532,7 +532,3 @@ Status:
 ```
 
 **Note**: If you make an existing resource invalid, the Ingress Controller will reject it.
-
-## Footnotes
-
-[^1]: Capabilities labeled in preview status are fully supported.

docs/content/installation/installation-with-helm.md

@@ -185,7 +185,8 @@ The following tables lists the configurable parameters of the NGINX Ingress cont
 |``controller.setAsDefaultIngress`` | New Ingresses without an ingressClassName field specified will be assigned the class specified in `controller.ingressClass`. | false |
 |``controller.watchNamespace`` | Namespace to watch for Ingress resources. By default the Ingress controller watches all namespaces. | "" |
 |``controller.enableCustomResources`` | Enable the custom resources. | true |
-|``controller.enablePreviewPolicies`` | Enable preview policies. | false |
+|``controller.enablePreviewPolicies`` | Enable preview policies. This parameter is deprecated. To enable OIDC Policies please use ``controller.enableOIDC`` instead. | false |
+|``controller.enableOIDC`` | Enable OIDC policies. | false |
 |``controller.enableTLSPassthrough`` | Enable TLS Passthrough on port 443. Requires ``controller.enableCustomResources``. | false |
 |``controller.globalConfiguration.create`` | Creates the GlobalConfiguration custom resource. Requires ``controller.enableCustomResources``. | false |
 |``controller.globalConfiguration.spec`` | The spec of the GlobalConfiguration for defining the global configuration parameters of the Ingress Controller. | {} |

docs/content/installation/installation-with-manifests.md

@@ -90,8 +90,6 @@ If you would like to use the TCP and UDP load balancing features of the Ingress
     $ kubectl apply -f common/crds/k8s.nginx.org_globalconfigurations.yaml
     ```
 
-> **Feature Status**: The Policy resources are available as a preview feature[^1]: We might introduce some backward-incompatible changes to the resource definition. The feature is disabled by default.
-
 ### Resources for NGINX App Protect
 
 If you would like to use the App Protect module, create the following additional resources:
@@ -257,7 +255,3 @@ $ kubectl get pods --namespace=nginx-ingress
     ```
     $ kubectl delete -f common/crds/
     ```
-
-## Footnotes
-
-[^1]: Capabilities labeled in preview status are fully supported.

internal/configs/config_params.go

@@ -121,7 +121,7 @@ type StaticConfigParams struct {
 	MainAppProtectDosLoadModule    bool
 	PodName                        string
 	EnableLatencyMetrics           bool
-	EnablePreviewPolicies          bool
+	EnableOIDC                     bool
 	SSLRejectHandshake             bool
 	EnableCertManager              bool
 }

internal/configs/configmaps.go

@@ -593,7 +593,7 @@ func GenerateNginxMainConfig(staticCfgParams *StaticConfigParams, config *Config
 		InternalRouteServer:                staticCfgParams.EnableInternalRoutes,
 		InternalRouteServerName:            staticCfgParams.PodName,
 		LatencyMetrics:                     staticCfgParams.EnableLatencyMetrics,
-		PreviewPolicies:                    staticCfgParams.EnablePreviewPolicies,
+		OIDC:                               staticCfgParams.EnableOIDC,
 	}
 	return nginxCfg
 }

internal/configs/version1/config.go

@@ -212,7 +212,7 @@ type MainConfig struct {
 	InternalRouteServer                bool
 	InternalRouteServerName            string
 	LatencyMetrics                     bool
-	PreviewPolicies                    bool
+	OIDC                               bool
 }
 
 // NewUpstreamWithDefaultServer creates an upstream with the default server.

internal/configs/version1/nginx-plus.tmpl

@@ -26,7 +26,7 @@ load_module modules/ngx_http_app_protect_dos_module.so;
 {{$value}}{{end}}
 {{- end}}
 
-{{if .PreviewPolicies}}
+{{if .OIDC}}
 load_module modules/ngx_http_js_module.so;
 {{- end}}
 
@@ -137,7 +137,7 @@ http {
     {{if .ResolverTimeout}}resolver_timeout {{.ResolverTimeout}};{{end}}
     {{end}}
 
-    {{if .PreviewPolicies}}
+    {{if .OIDC}}
     include oidc/oidc_common.conf;
     {{- end}}
 

internal/k8s/controller.go

@@ -148,7 +148,7 @@ type LoadBalancerController struct {
 	controllerNamespace           string
 	wildcardTLSSecret             string
 	areCustomResourcesEnabled     bool
-	enablePreviewPolicies         bool
+	enableOIDC                    bool
 	metricsCollector              collectors.ControllerCollector
 	globalConfigurationValidator  *validation.GlobalConfigurationValidator
 	transportServerValidator      *validation.TransportServerValidator
@@ -192,7 +192,7 @@ type NewLoadBalancerControllerInput struct {
 	ConfigMaps                   string
 	GlobalConfiguration          string
 	AreCustomResourcesEnabled    bool
-	EnablePreviewPolicies        bool
+	EnableOIDC                   bool
 	MetricsCollector             collectors.ControllerCollector
 	GlobalConfigurationValidator *validation.GlobalConfigurationValidator
 	TransportServerValidator     *validation.TransportServerValidator
@@ -227,7 +227,7 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc
 		controllerNamespace:          input.ControllerNamespace,
 		wildcardTLSSecret:            input.WildcardTLSSecret,
 		areCustomResourcesEnabled:    input.AreCustomResourcesEnabled,
-		enablePreviewPolicies:        input.EnablePreviewPolicies,
+		enableOIDC:                   input.EnableOIDC,
 		metricsCollector:             input.MetricsCollector,
 		globalConfigurationValidator: input.GlobalConfigurationValidator,
 		transportServerValidator:     input.TransportServerValidator,
@@ -893,7 +893,7 @@ func (lbc *LoadBalancerController) syncPolicy(task task) {
 
 	if polExists && lbc.HasCorrectIngressClass(obj) {
 		pol := obj.(*conf_v1.Policy)
-		err := validation.ValidatePolicy(pol, lbc.isNginxPlus, lbc.enablePreviewPolicies, lbc.appProtectEnabled)
+		err := validation.ValidatePolicy(pol, lbc.isNginxPlus, lbc.enableOIDC, lbc.appProtectEnabled)
 		if err != nil {
 			msg := fmt.Sprintf("Policy %v/%v is invalid and was rejected: %v", pol.Namespace, pol.Name, err)
 			lbc.recorder.Eventf(pol, api_v1.EventTypeWarning, "Rejected", msg)
@@ -2093,7 +2093,7 @@ func (lbc *LoadBalancerController) updatePoliciesStatus() error {
 	for _, obj := range lbc.policyLister.List() {
 		pol := obj.(*conf_v1.Policy)
 
-		err := validation.ValidatePolicy(pol, lbc.isNginxPlus, lbc.enablePreviewPolicies, lbc.appProtectEnabled)
+		err := validation.ValidatePolicy(pol, lbc.isNginxPlus, lbc.enableOIDC, lbc.appProtectEnabled)
 		if err != nil {
 			msg := fmt.Sprintf("Policy %v/%v is invalid and was rejected: %v", pol.Namespace, pol.Name, err)
 			err = lbc.statusUpdater.UpdatePolicyStatus(pol, conf_v1.StateInvalid, "Rejected", msg)
@@ -2641,7 +2641,7 @@ func (lbc *LoadBalancerController) getAllPolicies() []*conf_v1.Policy {
 	for _, obj := range lbc.policyLister.List() {
 		pol := obj.(*conf_v1.Policy)
 
-		err := validation.ValidatePolicy(pol, lbc.isNginxPlus, lbc.enablePreviewPolicies, lbc.appProtectEnabled)
+		err := validation.ValidatePolicy(pol, lbc.isNginxPlus, lbc.enableOIDC, lbc.appProtectEnabled)
 		if err != nil {
 			glog.V(3).Infof("Skipping invalid Policy %s/%s: %v", pol.Namespace, pol.Name, err)
 			continue
@@ -2683,7 +2683,7 @@ func (lbc *LoadBalancerController) getPolicies(policies []conf_v1.PolicyReferenc
 			continue
 		}
 
-		err = validation.ValidatePolicy(policy, lbc.isNginxPlus, lbc.enablePreviewPolicies, lbc.appProtectEnabled)
+		err = validation.ValidatePolicy(policy, lbc.isNginxPlus, lbc.enableOIDC, lbc.appProtectEnabled)
 		if err != nil {
 			errors = append(errors, fmt.Errorf("Policy %s is invalid: %w", policyKey, err))
 			continue

pkg/apis/configuration/v1/types.go

@@ -433,7 +433,6 @@ type EgressMTLS struct {
 }
 
 // OIDC defines an Open ID Connect policy.
-// policy status: preview
 type OIDC struct {
 	AuthEndpoint  string `json:"authEndpoint"`
 	TokenEndpoint string `json:"tokenEndpoint"`

pkg/apis/configuration/validation/policy.go

@@ -14,12 +14,12 @@ import (
 )
 
 // ValidatePolicy validates a Policy.
-func ValidatePolicy(policy *v1.Policy, isPlus, enablePreviewPolicies, enableAppProtect bool) error {
-	allErrs := validatePolicySpec(&policy.Spec, field.NewPath("spec"), isPlus, enablePreviewPolicies, enableAppProtect)
+func ValidatePolicy(policy *v1.Policy, isPlus, enableOIDC, enableAppProtect bool) error {
+	allErrs := validatePolicySpec(&policy.Spec, field.NewPath("spec"), isPlus, enableOIDC, enableAppProtect)
 	return allErrs.ToAggregate()
 }
 
-func validatePolicySpec(spec *v1.PolicySpec, fieldPath *field.Path, isPlus, enablePreviewPolicies, enableAppProtect bool) field.ErrorList {
+func validatePolicySpec(spec *v1.PolicySpec, fieldPath *field.Path, isPlus, enableOIDC, enableAppProtect bool) field.ErrorList {
 	allErrs := field.ErrorList{}
 
 	fieldCount := 0
@@ -54,9 +54,9 @@ func validatePolicySpec(spec *v1.PolicySpec, fieldPath *field.Path, isPlus, enab
 	}
 
 	if spec.OIDC != nil {
-		if !enablePreviewPolicies {
+		if !enableOIDC {
 			allErrs = append(allErrs, field.Forbidden(fieldPath.Child("oidc"),
-				"oidc is a preview policy. Preview policies must be enabled to use via cli argument -enable-preview-policies"))
+				"OIDC must be enabled via cli argument -enable-oidc to use OIDC policy"))
 		}
 		if !isPlus {
 			return append(allErrs, field.Forbidden(fieldPath.Child("oidc"), "OIDC is only supported in NGINX Plus"))