revert: deprecate and replace flag family --publish-service, etc. with --ingress-service (#4765 & #4820) (#4921)

Adjust existing code and reverted (partially - leave adjustments in tests) commits:
- ad292cd
- 15a036f

Author: programmer04

CHANGELOG.md

@@ -165,12 +165,6 @@ Adding a new version? You'll need three changes:
   [#4737](https://github.com/Kong/kubernetes-ingress-controller/pull/4737)
 - Telemetry now reports the router flavor.
   [#4762](https://github.com/Kong/kubernetes-ingress-controller/pull/4762)
-- The following flags were renamed and marked as deprecated
-  - `--publish-service` to `--ingress-service`
-  - `--publish-status-address` to `--ingress-address`
-  - `--publish-service-udp` to `--ingress-service-udp`
-  - `--publish-status-address-udp` to `--ingress-address-udp`
-  [#4765](https://github.com/Kong/kubernetes-ingress-controller/pull/4765)
 - Support Query Parameter matching of `HTTPRoute` when expression router enabled.
   [#4780](https://github.com/Kong/kubernetes-ingress-controller/pull/4780)
 

CONTRIBUTING.md

@@ -157,7 +157,7 @@ export POD_NAMESPACE=kong
 
 go run -tags gcp ./internal/cmd/main.go \
 --kubeconfig ~/.kube/config \
---ingress-service=kong/kong-proxy \
+--publish-service=kong/kong-proxy \
 --apiserver-host=http://localhost:8002 \
 --kong-admin-url https://localhost:8444 \
 --kong-admin-tls-skip-verify true

Makefile

@@ -545,8 +545,8 @@ debug: install _ensure-namespace
 	$(DLV) debug ./internal/cmd/main.go -- \
 		--anonymous-reports=false \
 		--kong-admin-url $(KONG_ADMIN_URL) \
-		--ingress-service $(KONG_NAMESPACE)/$(KONG_PROXY_SERVICE) \
-		--ingress-service-udp $(KONG_NAMESPACE)/$(KONG_PROXY_UDP_SERVICE) \
+		--publish-service $(KONG_NAMESPACE)/$(KONG_PROXY_SERVICE) \
+		--publish-service-udp $(KONG_NAMESPACE)/$(KONG_PROXY_UDP_SERVICE) \
 		--kubeconfig $(KUBECONFIG) \
 		--feature-gates=$(KONG_CONTROLLER_FEATURE_GATES)
 
@@ -618,8 +618,8 @@ _run:
 	go run ./internal/cmd/main.go \
 		--anonymous-reports=false \
 		--kong-admin-url $(KONG_ADMIN_URL) \
-		--ingress-service $(KONG_NAMESPACE)/$(KONG_PROXY_SERVICE) \
-		--ingress-service-udp $(KONG_NAMESPACE)/$(KONG_PROXY_UDP_SERVICE) \
+		--publish-service $(KONG_NAMESPACE)/$(KONG_PROXY_SERVICE) \
+		--publish-service-udp $(KONG_NAMESPACE)/$(KONG_PROXY_UDP_SERVICE) \
 		--kubeconfig $(KUBECONFIG) \
 		--feature-gates=$(KONG_CONTROLLER_FEATURE_GATES)
 

docs/cli-arguments.md

@@ -33,11 +33,7 @@
 | `--gateway-api-controller-name` | `string` | The controller name to match on Gateway API resources. | `konghq.com/kic-gateway-controller` |
 | `--gateway-discovery-dns-strategy` | `dns-strategy` | DNS strategy to use when creating Gateway's Admin API addresses. One of: ip, service, pod. | `"ip"` |
 | `--health-probe-bind-address` | `string` | The address the probe endpoint binds to. | `:10254` |
-| `--ingress-address` | `stringSlice` | User-provided address(es) in comma-separated string format (or specify this flag multiple times), for use in lieu of "publish-service" when that Service lacks useful address information (for example, in bare-metal environments). | `[]` |
-| `--ingress-address-udp` | `stringSlice` | User-provided address(es) in comma-separated string format (or specify this flag multiple times), for use in lieu of "publish-service-udp" when that Service lacks useful address information. | `[]` |
 | `--ingress-class` | `string` | Name of the ingress class to route through this controller. | `kong` |
-| `--ingress-service` | `namespacedName` | Service fronting Ingress resources in "namespace/name" format. The controller will update Ingress status information with this Service's endpoints. |  |
-| `--ingress-service-udp` | `namespacedName` | Service fronting UDP routing resources in "namespace/name" format. The controller will update UDP route status information with this Service's endpoints. If omitted, the same Service will be used for both TCP and UDP routes. |  |
 | `--kong-admin-ca-cert` | `string` | PEM-encoded CA certificate to verify Kong's Admin SSL certificate. |  |
 | `--kong-admin-ca-cert-file` | `string` | Path to PEM-encoded CA certificate file to verify Kong's Admin SSL certificate. |  |
 | `--kong-admin-concurrency` | `int` | Max number of concurrent requests sent to Kong's Admin API. | `10` |
@@ -75,6 +71,10 @@
 | `--profiling` | `bool` | Enable profiling via web interface host:10256/debug/pprof/. | `false` |
 | `--proxy-sync-seconds` | `float32` | Define the rate (in seconds) in which configuration updates will be applied to the Kong Admin API. | `3` |
 | `--proxy-timeout-seconds` | `float32` | Sets the timeout (in seconds) for all requests to Kong's Admin API. | `30` |
+| `--publish-service` | `namespacedName` | Service fronting Ingress resources in "namespace/name" format. The controller will update Ingress status information with this Service's endpoints. |  |
+| `--publish-service-udp` | `namespacedName` | Service fronting UDP routing resources in "namespace/name" format. The controller will update UDP route status information with this Service's endpoints. If omitted, the same Service will be used for both TCP and UDP routes. |  |
+| `--publish-status-address` | `stringSlice` | User-provided addresses in comma-separated string format, for use in lieu of "publish-service" when that Service lacks useful address information (for example, in bare-metal environments). | `[]` |
+| `--publish-status-address-udp` | `stringSlice` | User-provided address CSV, for use in lieu of "publish-service-udp" when that Service lacks useful address information. | `[]` |
 | `--skip-ca-certificates` | `bool` | Disable syncing CA certificate syncing (for use with multi-workspace environments). | `false` |
 | `--sync-period` | `duration` | Relist and confirm cloud resources this often. | `48h0m0s` |
 | `--term-delay` | `duration` | The time delay to sleep before SIGTERM or SIGINT will shut down the Ingress Controller. | `0s` |

internal/cmd/rootcmd/rootcmd_test.go

@@ -16,30 +16,24 @@ func TestRootCmd(t *testing.T) {
 		require.NoError(t, rootCmd.PersistentPreRunE(rootCmd, os.Args[:0]))
 	})
 
-	t.Run("binding environment variables succeeds when flag validation passes", func(t *testing.T) {
-		t.Setenv("CONTROLLER_INGRESS_SERVICE", "namespace/servicename")
+	t.Run("root command succeeds when correct flags where provided", func(t *testing.T) {
 		var cfg manager.Config
 		rootCmd := GetRootCmd(&cfg)
-		require.NoError(t, rootCmd.PersistentPreRunE(rootCmd, os.Args[:0]))
+		require.NoError(t, rootCmd.PersistentPreRunE(rootCmd,
+			append(os.Args[:0],
+				"--publish-service", "namespace/servicename",
+			),
+		))
 	})
 
-	t.Run("binding environment variables fails when flag validation fails", func(t *testing.T) {
-		t.Setenv("CONTROLLER_INGRESS_SERVICE", "servicename")
-		var cfg manager.Config
-		rootCmd := GetRootCmd(&cfg)
-		require.Error(t, rootCmd.PersistentPreRunE(rootCmd, os.Args[:0]),
-			"binding env vars should fail because a non namespaced name of ingress service was provided",
-		)
-	})
-
-	t.Run("binding deprecated environment variables succeeds when flag validation passes", func(t *testing.T) {
+	t.Run("binding environment variables succeeds when flag validation passes", func(t *testing.T) {
 		t.Setenv("CONTROLLER_PUBLISH_SERVICE", "namespace/servicename")
 		var cfg manager.Config
 		rootCmd := GetRootCmd(&cfg)
 		require.NoError(t, rootCmd.PersistentPreRunE(rootCmd, os.Args[:0]))
 	})
 
-	t.Run("binding deprecated environment variables fails when flag validation fails", func(t *testing.T) {
+	t.Run("binding environment variables fails when flag validation fails", func(t *testing.T) {
 		t.Setenv("CONTROLLER_PUBLISH_SERVICE", "servicename")
 		var cfg manager.Config
 		rootCmd := GetRootCmd(&cfg)

internal/controllers/gateway/gateway_controller.go

@@ -56,8 +56,8 @@ type GatewayReconciler struct { //nolint:revive
 
 	ReferenceIndexers ctrlref.CacheIndexers
 
-	IngressServiceRef    k8stypes.NamespacedName
-	IngressServiceUDPRef mo.Option[k8stypes.NamespacedName]
+	PublishServiceRef    k8stypes.NamespacedName
+	PublishServiceUDPRef mo.Option[k8stypes.NamespacedName]
 
 	// If enableReferenceGrant is true, controller will watch ReferenceGrants
 	// to invalidate or allow cross-namespace TLSConfigs in gateways.
@@ -67,9 +67,9 @@ type GatewayReconciler struct { //nolint:revive
 
 // SetupWithManager sets up the controller with the Manager.
 func (r *GatewayReconciler) SetupWithManager(mgr ctrl.Manager) error {
-	// Verify that the Ingress Service was configured properly.
-	if r.IngressServiceRef.Name == "" || r.IngressServiceRef.Namespace == "" {
-		return fmt.Errorf("ingress service must be configured")
+	// verify that the PublishService was configured properly
+	if r.PublishServiceRef.Name == "" || r.PublishServiceRef.Namespace == "" {
+		return fmt.Errorf("publish service must be configured")
 	}
 
 	// We're verifying whether ReferenceGrant CRD is installed at setup of the GatewayReconciler
@@ -298,13 +298,13 @@ func (r *GatewayReconciler) listGatewaysForHTTPRoute(_ context.Context, obj clie
 }
 
 // isGatewayService is a watch predicate that filters out events for objects that aren't
-// the gateway service referenced by --ingress-service or --ingress-service-udp.
+// the gateway service referenced by --publish-service or --publish-service-udp.
 func (r *GatewayReconciler) isGatewayService(obj client.Object) bool {
-	isIngressService := fmt.Sprintf("%s/%s", obj.GetNamespace(), obj.GetName()) == r.IngressServiceRef.String()
-	isIngressServiceUDP := r.IngressServiceUDPRef.IsPresent() &&
-		fmt.Sprintf("%s/%s", obj.GetNamespace(), obj.GetName()) == r.IngressServiceUDPRef.MustGet().String()
+	isPublishService := fmt.Sprintf("%s/%s", obj.GetNamespace(), obj.GetName()) == r.PublishServiceRef.String()
+	isUDPPublishService := r.PublishServiceUDPRef.IsPresent() &&
+		fmt.Sprintf("%s/%s", obj.GetNamespace(), obj.GetName()) == r.PublishServiceUDPRef.MustGet().String()
 
-	return isIngressService || isIngressServiceUDP
+	return isPublishService || isUDPPublishService
 }
 
 func referenceGrantHasGatewayFrom(obj client.Object) bool {
@@ -441,10 +441,10 @@ func (r *GatewayReconciler) reconcileUnmanagedGateway(ctx context.Context, log l
 	// enforce the service reference as the annotation value for the key UnmanagedGateway.
 	debug(log, gateway, "initializing admin service annotation if unset")
 	if len(annotations.ExtractGatewayPublishService(gateway.Annotations)) == 0 {
-		services := []string{r.IngressServiceRef.String()}
+		services := []string{r.PublishServiceRef.String()}
 
 		// UDP service is optional.
-		if udpRef, ok := r.IngressServiceUDPRef.Get(); ok {
+		if udpRef, ok := r.PublishServiceUDPRef.Get(); ok {
 			services = append(services, udpRef.String())
 		}
 
@@ -459,7 +459,7 @@ func (r *GatewayReconciler) reconcileUnmanagedGateway(ctx context.Context, log l
 	serviceRefs := annotations.ExtractGatewayPublishService(gateway.Annotations)
 	// Validation check of the Gateway to ensure that the ingress service is actually available
 	// in the cluster. If it is not the object will be requeued until it exists (or is otherwise retrievable).
-	debug(log, gateway, "gathering the gateway ingress service") // This will also be done by the validating webhook, this is a fallback.
+	debug(log, gateway, "gathering the gateway publish service") // this will also be done by the validating webhook, this is a fallback
 	var gatewayServices []*corev1.Service
 	for _, ref := range serviceRefs {
 		r.Log.V(util.DebugLevel).Info("determining service for ref", "ref", ref)
@@ -503,7 +503,7 @@ func (r *GatewayReconciler) reconcileUnmanagedGateway(ctx context.Context, log l
 	// from the Kubernetes Service which will also give us all the L4 information about the proxy. From there
 	// we can use that L4 information to derive the higher level TLS and HTTP,GRPC, e.t.c. information from
 	// the data-plane's metadata.
-	debug(log, gateway, "determining listener configurations from ingress services")
+	debug(log, gateway, "determining listener configurations from publish services")
 	var combinedAddresses []gatewayapi.GatewayAddress
 	var combinedListeners []gatewayapi.Listener
 	for _, svc := range gatewayServices {
@@ -596,22 +596,22 @@ func init() {
 	}
 }
 
-// determineServiceForGateway provides the "ingress service" (aka the proxy Service) object which
+// determineServiceForGateway provides the "publish service" (aka the proxy Service) object which
 // will be used to populate unmanaged gateways.
 func (r *GatewayReconciler) determineServiceForGateway(ctx context.Context, ref string) (*corev1.Service, error) {
-	// Currently the gateway controller ONLY supports service references that correspond with the --ingress-service
+	// currently the gateway controller ONLY supports service references that correspond with the --publish-service
 	// provided to the controller manager via flags when operating on unmanaged gateways. This constraint may
 	// be loosened in later iterations if there is need.
 
 	var name k8stypes.NamespacedName
 	switch {
-	case ref == r.IngressServiceRef.String():
-		name = r.IngressServiceRef
-	case r.IngressServiceUDPRef.IsPresent() && ref == r.IngressServiceUDPRef.MustGet().String():
-		name = r.IngressServiceUDPRef.MustGet()
+	case ref == r.PublishServiceRef.String():
+		name = r.PublishServiceRef
+	case r.PublishServiceUDPRef.IsPresent() && ref == r.PublishServiceUDPRef.MustGet().String():
+		name = r.PublishServiceUDPRef.MustGet()
 	default:
 		return nil, fmt.Errorf("service ref %s did not match controller manager ref %s or %s",
-			ref, r.IngressServiceRef.String(), r.IngressServiceUDPRef.OrEmpty())
+			ref, r.PublishServiceRef.String(), r.PublishServiceUDPRef.OrEmpty())
 	}
 
 	// retrieve the service for the kong gateway

internal/manager/config.go

@@ -85,11 +85,11 @@ type Config struct {
 	GatewayAPIControllerName string
 	Impersonate              string
 
-	// Ingress status.
-	IngressServiceUDP   OptionalNamespacedName
-	IngressService      OptionalNamespacedName
-	IngressAddresses    []string
-	IngressAddressesUDP []string
+	// Ingress status
+	PublishServiceUDP       OptionalNamespacedName
+	PublishService          OptionalNamespacedName
+	PublishStatusAddress    []string
+	PublishStatusAddressUDP []string
 
 	UpdateStatus                bool
 	UpdateStatusQueueBufferSize int
@@ -202,16 +202,16 @@ func (c *Config) FlagSet() *pflag.FlagSet {
 		`Namespace(s) to watch for Kubernetes resources. Defaults to all namespaces. To watch multiple namespaces, use a comma-separated list of namespaces.`)
 
 	// Ingress status
-	flagSet.Var(flags.NewValidatedValue(&c.IngressService, namespacedNameFromFlagValue, nnTypeNameOverride), "ingress-service",
+	flagSet.Var(flags.NewValidatedValue(&c.PublishService, namespacedNameFromFlagValue, nnTypeNameOverride), "publish-service",
 		`Service fronting Ingress resources in "namespace/name" format. The controller will update Ingress status information with this Service's endpoints.`)
-	flagSet.StringSliceVar(&c.IngressAddresses, "ingress-address", []string{},
-		`User-provided address(es) in comma-separated string format (or specify this flag multiple times), for use in lieu of "publish-service" `+
+	flagSet.StringSliceVar(&c.PublishStatusAddress, "publish-status-address", []string{},
+		`User-provided addresses in comma-separated string format, for use in lieu of "publish-service" `+
 			`when that Service lacks useful address information (for example, in bare-metal environments).`)
-	flagSet.Var(flags.NewValidatedValue(&c.IngressServiceUDP, namespacedNameFromFlagValue, nnTypeNameOverride), "ingress-service-udp", `Service fronting UDP routing resources in `+
+	flagSet.Var(flags.NewValidatedValue(&c.PublishServiceUDP, namespacedNameFromFlagValue, nnTypeNameOverride), "publish-service-udp", `Service fronting UDP routing resources in `+
 		`"namespace/name" format. The controller will update UDP route status information with this Service's `+
 		`endpoints. If omitted, the same Service will be used for both TCP and UDP routes.`)
-	flagSet.StringSliceVar(&c.IngressAddressesUDP, "ingress-address-udp", []string{},
-		`User-provided address(es) in comma-separated string format (or specify this flag multiple times), for use in lieu of "publish-service-udp" when that Service lacks useful address information.`)
+	flagSet.StringSliceVar(&c.PublishStatusAddressUDP, "publish-status-address-udp", []string{},
+		`User-provided address CSV, for use in lieu of "publish-service-udp" when that Service lacks useful address information.`)
 
 	flagSet.BoolVar(&c.UpdateStatus, "update-status", true,
 		`Indicates if the ingress controller should update the status of resources (e.g. IP/Hostname for v1.Ingress, e.t.c.)`)
@@ -269,18 +269,30 @@ func (c *Config) FlagSet() *pflag.FlagSet {
 	flagSet.DurationVar(&c.Konnect.RefreshNodePeriod, "konnect-refresh-node-period", konnect.DefaultRefreshNodePeriod, "Period of uploading status of KIC and controlled kong gateway instances")
 
 	// Deprecated flags
-	flagSet.Var(flags.NewValidatedValue(&c.IngressService, namespacedNameFromFlagValue, nnTypeNameOverride), "publish-service", "Use --ingress-service instead")
-	_ = flagSet.MarkDeprecated("publish-service", "Use --ingress-service instead")
-	flagSet.Var(flags.NewValidatedValue(&c.IngressServiceUDP, namespacedNameFromFlagValue, nnTypeNameOverride), "publish-service-udp", "Use --ingress-service-udp instead")
-	_ = flagSet.MarkDeprecated("publish-service-udp", "Use --ingress-service-udp instead")
-	flagSet.StringSliceVar(&c.IngressAddresses, "publish-status-address", []string{}, "")
-	_ = flagSet.MarkDeprecated("publish-status-address", "Use --ingress-address instead")
-	flagSet.StringSliceVar(&c.IngressAddressesUDP, "publish-status-address-udp", []string{}, "")
-	_ = flagSet.MarkDeprecated("publish-status-address-udp", "Use --ingress-address-udp instead")
-
 	flagSet.StringVar(&c.Konnect.ControlPlaneID, "konnect-runtime-group-id", "", "Use --konnect-control-plane-id instead.")
 	_ = flagSet.MarkDeprecated("konnect-runtime-group-id", "Use --konnect-control-plane-id instead.")
 
+	_ = flagSet.Float32("sync-rate-limit", dataplane.DefaultSyncSeconds, "Use --proxy-sync-seconds instead")
+	_ = flagSet.MarkDeprecated("sync-rate-limit", "Use --proxy-sync-seconds instead")
+
+	_ = flagSet.Int("stderrthreshold", 0, "Has no effect and will be removed in future releases (see github issue #1297)")
+	_ = flagSet.MarkDeprecated("stderrthreshold", "Has no effect and will be removed in future releases (see github issue #1297)")
+
+	_ = flagSet.Bool("update-status-on-shutdown", false, "No longer has any effect and will be removed in a later release (see github issue #1304)")
+	_ = flagSet.MarkDeprecated("update-status-on-shutdown", "No longer has any effect and will be removed in a later release (see github issue #1304)")
+
+	_ = flagSet.String("kong-custom-entities-secret", "", "Will be removed in next major release.")
+	_ = flagSet.MarkDeprecated("kong-custom-entities-secret", "Will be removed in next major release.")
+
+	_ = flagSet.Bool("leader-elect", false, "DEPRECATED as of 2.1.0: leader election behavior is determined automatically based on the Kong database setting and this flag has no effect")
+	_ = flagSet.MarkDeprecated("leader-elect", "DEPRECATED as of 2.1.0: leader election behavior is determined automatically based on the Kong database setting and this flag has no effect")
+
+	_ = flagSet.Bool("enable-controller-ingress-extensionsv1beta1", true, "DEPRECATED: Enable the extensions/v1beta1 Ingress controller.")
+	_ = flagSet.MarkDeprecated("enable-controller-ingress-extensionsv1beta1", "DEPRECATED: Enable the extensions/v1beta1 Ingress controller.")
+
+	_ = flagSet.Bool("enable-controller-ingress-networkingv1beta1", true, "Enable the networking.k8s.io/v1beta1 Ingress controller.")
+	_ = flagSet.MarkDeprecated("enable-controller-ingress-networkingv1beta1", "Enable the networking.k8s.io/v1beta1 Ingress controller.")
+
 	c.flagSet = flagSet
 	return flagSet
 }