Skip to content

Commit

Permalink
feat: allow configuring cluster domain and use FQDN for upstream serv…
Browse files Browse the repository at this point in the history
…ice targets (#6697)

* feat: allow configuring cluster domain and use FQDN for upstream service targets

* Update CHANGELOG.md

* chore: extract manager's consts to a separate consts package

* chore: generate CLI args

---------

Co-authored-by: Mattia Lavacca <[email protected]>
  • Loading branch information
pmalek and mlavacca authored Dec 3, 2024
1 parent c9d315a commit fbfdbeb
Show file tree
Hide file tree
Showing 10 changed files with 121 additions and 45 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ Adding a new version? You'll need three changes:
- Log `Object requested backendRef to target, but it does not exist, skipping...`
as `DEBUG` instead of `ERROR`, enhance `HTTPRoute` status with detailed message.
[#6746](https://github.com/Kong/kubernetes-ingress-controller/pull/6746)
- From now on, upstreams produced by KIC from `Service`s that are configured as
upstream services (either by `ingress.kubernetes.io/service-upstream` annotation
or through `IngressClassNamespacedParameters`'s `serviceUpstream` field), will use
a FQDN with a default cluster domain of `""`.
Users can override the default by setting the `--cluster-domain` flag.
[#6697](https://github.com/Kong/kubernetes-ingress-controller/pull/6697)

### Fixed

Expand Down
1 change: 1 addition & 0 deletions docs/cli-arguments.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
| `--apiserver-host` | `string` | The Kubernetes API server URL. If not set, the controller will use cluster config discovery. | |
| `--apiserver-qps` | `int` | The Kubernetes API RateLimiter maximum queries per second. | `100` |
| `--cache-sync-timeout` | `duration` | The time limit set to wait for syncing controllers' caches. Set to 0 to use default from controller-runtime. | `2m0s` |
| `--cluster-domain` | `string` | The cluster domain. This is used e.g. in generating addresses for upstream services. | |
| `--dump-config` | `bool` | Enable config dumps via web interface host:10256/debug/config. | `false` |
| `--dump-sensitive-config` | `bool` | Include credentials and TLS secrets in configs exposed with --dump-config flag. | `false` |
| `--election-id` | `string` | Election id to use for status update. | `5b374a9e.konghq.com` |
Expand Down
3 changes: 2 additions & 1 deletion internal/dataplane/kong_client_golden_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/sendconfig"
"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/translator"
"github.com/kong/kubernetes-ingress-controller/v3/internal/diagnostics"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/consts"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/featuregates"
"github.com/kong/kubernetes-ingress-controller/v3/internal/store"
"github.com/kong/kubernetes-ingress-controller/v3/test/mocks"
Expand Down Expand Up @@ -259,7 +260,7 @@ func runKongClientGoldenTest(t *testing.T, tc kongClientGoldenTestCase) {
// Create the translator.
logger := zapr.NewLogger(zap.NewNop())
s := store.New(cacheStores, "kong", logger)
p, err := translator.NewTranslator(logger, s, "", tc.featureFlags, fakeSchemaServiceProvier{})
p, err := translator.NewTranslator(logger, s, "", tc.featureFlags, fakeSchemaServiceProvier{}, consts.DefaultClusterDomain)
require.NoError(t, err, "failed creating translator")

// Start a mock Admin API server and create an Admin API client for inspecting the configuration.
Expand Down
12 changes: 9 additions & 3 deletions internal/dataplane/translator/translate_upstreams.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (t *Translator) getUpstreams(serviceMap map[string]kongstate.Service) ([]ko
serviceMap[serviceName] = service

// get the new targets for this backend service
newTargets := getServiceEndpoints(t.logger, t.storer, k8sService, port)
newTargets := getServiceEndpoints(t.logger, t.storer, k8sService, port, t.clusterDomain)

if len(newTargets) == 0 {
t.logger.V(logging.InfoLevel).Info("No targets could be found for kubernetes service",
Expand Down Expand Up @@ -195,6 +195,7 @@ func getServiceEndpoints(
s store.Storer,
svc *corev1.Service,
servicePort *corev1.ServicePort,
clusterDomain string,
) []kongstate.Target {
logger = logger.WithValues(
"service_name", svc.Name,
Expand All @@ -219,7 +220,7 @@ func getServiceEndpoints(
// Check all protocols for associated endpoints.
endpoints := []util.Endpoint{}
for protocol := range protocols {
newEndpoints := getEndpoints(logger, svc, servicePort, protocol, s.GetEndpointSlicesForService, isSvcUpstream)
newEndpoints := getEndpoints(logger, svc, servicePort, protocol, s.GetEndpointSlicesForService, isSvcUpstream, clusterDomain)
endpoints = append(endpoints, newEndpoints...)
}
if len(endpoints) == 0 {
Expand Down Expand Up @@ -257,6 +258,7 @@ func getEndpoints(
proto corev1.Protocol,
getEndpointSlices func(string, string) ([]*discoveryv1.EndpointSlice, error),
isSvcUpstream bool,
clusterDomain string,
) []util.Endpoint {
if service == nil || port == nil {
return []util.Endpoint{}
Expand All @@ -265,9 +267,13 @@ func getEndpoints(
// If service is an upstream service...
if isSvcUpstream || annotations.HasServiceUpstreamAnnotation(service.Annotations) {
// ... return its address as the only endpoint.
svcDomainName := service.Name + "." + service.Namespace + ".svc"
if clusterDomain != "" {
svcDomainName += "." + clusterDomain
}
return []util.Endpoint{
{
Address: service.Name + "." + service.Namespace + ".svc",
Address: svcDomainName,
Port: fmt.Sprint(port.Port),
},
}
Expand Down
4 changes: 4 additions & 0 deletions internal/dataplane/translator/translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ type Translator struct {

failuresCollector *failures.ResourceFailuresCollector
translatedObjectsCollector *ObjectsCollector

clusterDomain string
}

// NewTranslator produces a new Translator object provided a logging mechanism
Expand All @@ -107,6 +109,7 @@ func NewTranslator(
workspace string,
featureFlags FeatureFlags,
schemaServiceProvider SchemaServiceProvider,
clusterDomain string,
) (*Translator, error) {
failuresCollector := failures.NewResourceFailuresCollector(logger)

Expand All @@ -124,6 +127,7 @@ func NewTranslator(
schemaServiceProvider: schemaServiceProvider,
failuresCollector: failuresCollector,
translatedObjectsCollector: translatedObjectsCollector,
clusterDomain: clusterDomain,
}, nil
}

Expand Down
65 changes: 56 additions & 9 deletions internal/dataplane/translator/translator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/kong/kubernetes-ingress-controller/v3/internal/annotations"
dpconf "github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/config"
"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/kongstate"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/consts"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/featuregates"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/scheme"
"github.com/kong/kubernetes-ingress-controller/v3/internal/store"
Expand Down Expand Up @@ -3763,8 +3764,9 @@ func TestGetEndpoints(t *testing.T) {
port *corev1.ServicePort
proto corev1.Protocol
fn func(string, string) ([]*discoveryv1.EndpointSlice, error)
result []util.Endpoint
isServiceUpstream bool
clusterDomain string
result []util.Endpoint
}{
{
name: "no service should return 0 endpoints",
Expand Down Expand Up @@ -3897,6 +3899,44 @@ func TestGetEndpoints(t *testing.T) {
},
isServiceUpstream: true,
},
{
name: "a service with ingress.kubernetes.io/service-upstream annotation should return one endpoint properly mapping to provided custom domain",
svc: &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
Annotations: map[string]string{
"ingress.kubernetes.io/service-upstream": "true",
},
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeClusterIP,
Ports: []corev1.ServicePort{
{
Name: "default",
TargetPort: intstr.FromInt(80),
Port: 2080,
},
},
},
},
port: &corev1.ServicePort{
Name: "default",
TargetPort: intstr.FromInt(80),
Port: 2080,
},
proto: corev1.ProtocolTCP,
fn: func(string, string) ([]*discoveryv1.EndpointSlice, error) {
return []*discoveryv1.EndpointSlice{}, nil
},
clusterDomain: "acme.com",
result: []util.Endpoint{
{
Address: "foo.bar.svc.acme.com",
Port: "2080",
},
},
},
{
name: "should return no endpoints when there is an error searching for endpoints",
svc: &corev1.Service{
Expand Down Expand Up @@ -4123,8 +4163,11 @@ func TestGetEndpoints(t *testing.T) {

for _, testCase := range tests {
t.Run(testCase.name, func(t *testing.T) {
result := getEndpoints(zapr.NewLogger(zap.NewNop()), testCase.svc, testCase.port, testCase.proto, testCase.fn,
testCase.isServiceUpstream)
clusterDomain := consts.DefaultClusterDomain
if testCase.clusterDomain != "" {
clusterDomain = testCase.clusterDomain
}
result := getEndpoints(zapr.NewLogger(zap.NewNop()), testCase.svc, testCase.port, testCase.proto, testCase.fn, testCase.isServiceUpstream, clusterDomain)
require.Equal(t, testCase.result, result)
})
}
Expand Down Expand Up @@ -5060,12 +5103,16 @@ func (p fakeSchemaServiceProvier) GetSchemaService() kong.AbstractSchemaService
}

func mustNewTranslator(t *testing.T, storer store.Storer) *Translator {
p, err := NewTranslator(zapr.NewLogger(zap.NewNop()), storer, "", FeatureFlags{
// We'll assume these are true for all tests.
FillIDs: true,
ReportConfiguredKubernetesObjects: true,
KongServiceFacade: true,
}, fakeSchemaServiceProvier{},
logger := zapr.NewLogger(zap.NewNop())
p, err := NewTranslator(logger, storer, "",
FeatureFlags{
// We'll assume these are true for all tests.
FillIDs: true,
ReportConfiguredKubernetesObjects: true,
KongServiceFacade: true,
},
fakeSchemaServiceProvier{},
consts.DefaultClusterDomain,
)
require.NoError(t, err)
return p
Expand Down
13 changes: 8 additions & 5 deletions internal/manager/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/kong/kubernetes-ingress-controller/v3/internal/konnect"
"github.com/kong/kubernetes-ingress-controller/v3/internal/license"
cfgtypes "github.com/kong/kubernetes-ingress-controller/v3/internal/manager/config/types"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/consts"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/featuregates"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/flags"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/metadata"
Expand Down Expand Up @@ -96,6 +97,7 @@ type Config struct {
GatewayAPIControllerName string
Impersonate string
EmitKubernetesEvents bool
ClusterDomain string

// Ingress status
PublishServiceUDP OptionalNamespacedName
Expand Down Expand Up @@ -218,8 +220,8 @@ func (c *Config) FlagSet() *pflag.FlagSet {
flagSet.StringVar(&c.APIServerHost, "apiserver-host", "", `The Kubernetes API server URL. If not set, the controller will use cluster config discovery.`)
flagSet.IntVar(&c.APIServerQPS, "apiserver-qps", 100, "The Kubernetes API RateLimiter maximum queries per second.")
flagSet.IntVar(&c.APIServerBurst, "apiserver-burst", 300, "The Kubernetes API RateLimiter maximum burst queries per second.")
flagSet.StringVar(&c.MetricsAddr, "metrics-bind-address", fmt.Sprintf(":%v", MetricsPort), "The address the metric endpoint binds to.")
flagSet.StringVar(&c.ProbeAddr, "health-probe-bind-address", fmt.Sprintf(":%v", HealthzPort), "The address the probe endpoint binds to.")
flagSet.StringVar(&c.MetricsAddr, "metrics-bind-address", fmt.Sprintf(":%v", consts.MetricsPort), "The address the metric endpoint binds to.")
flagSet.StringVar(&c.ProbeAddr, "health-probe-bind-address", fmt.Sprintf(":%v", consts.HealthzPort), "The address the probe endpoint binds to.")
flagSet.Float32Var(&c.ProxySyncSeconds, "proxy-sync-seconds", dataplane.DefaultSyncSeconds,
"Define the rate (in seconds) in which configuration updates will be applied to the Kong Admin API.")
flagSet.DurationVar(&c.InitCacheSyncDuration, "init-cache-sync-duration", dataplane.DefaultCacheSyncWaitDuration, `The initial delay to wait for Kubernetes object caches to be synced before the initial configuration.`)
Expand All @@ -241,6 +243,7 @@ func (c *Config) FlagSet() *pflag.FlagSet {
flagSet.StringSliceVar(&c.WatchNamespaces, "watch-namespace", nil,
`Namespace(s) in comma-separated format (or specify this flag multiple times) to watch for Kubernetes resources. Defaults to all namespaces.`)
flagSet.BoolVar(&c.EmitKubernetesEvents, "emit-kubernetes-events", true, `Emit Kubernetes events for successful configuration applies, translation failures and configuration apply failures on managed objects.`)
flagSet.StringVar(&c.ClusterDomain, "cluster-domain", consts.DefaultClusterDomain, `The cluster domain. This is used e.g. in generating addresses for upstream services.`)

// Ingress status
flagSet.Var(flags.NewValidatedValue(&c.PublishService, namespacedNameFromFlagValue, nnTypeNameOverride), "publish-service",
Expand Down Expand Up @@ -297,10 +300,10 @@ func (c *Config) FlagSet() *pflag.FlagSet {
`Admission server PEM private key value. Mutually exclusive with --admission-webhook-key-file.`)

// Diagnostics
flagSet.BoolVar(&c.EnableProfiling, "profiling", false, fmt.Sprintf("Enable profiling via web interface host:%v/debug/pprof/.", DiagnosticsPort))
flagSet.BoolVar(&c.EnableConfigDumps, "dump-config", false, fmt.Sprintf("Enable config dumps via web interface host:%v/debug/config.", DiagnosticsPort))
flagSet.BoolVar(&c.EnableProfiling, "profiling", false, fmt.Sprintf("Enable profiling via web interface host:%v/debug/pprof/.", consts.DiagnosticsPort))
flagSet.BoolVar(&c.EnableConfigDumps, "dump-config", false, fmt.Sprintf("Enable config dumps via web interface host:%v/debug/config.", consts.DiagnosticsPort))
flagSet.BoolVar(&c.DumpSensitiveConfig, "dump-sensitive-config", false, "Include credentials and TLS secrets in configs exposed with --dump-config flag.")
flagSet.IntVar(&c.DiagnosticServerPort, "diagnostic-server-port", DiagnosticsPort, "The port to listen on for the profiling and config dump server.")
flagSet.IntVar(&c.DiagnosticServerPort, "diagnostic-server-port", consts.DiagnosticsPort, "The port to listen on for the profiling and config dump server.")
_ = flagSet.MarkHidden("diagnostic-server-port")

// Feature Gates (see FEATURE_GATES.md).
Expand Down
24 changes: 0 additions & 24 deletions internal/manager/consts.go

This file was deleted.

30 changes: 30 additions & 0 deletions internal/manager/consts/consts.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package consts

// -----------------------------------------------------------------------------
// Controller Manager - Constants & Vars
// -----------------------------------------------------------------------------

const (
// HealthzPort is the default port the manager's health service listens on.
// Changing this will result in a breaking change. Existing deployments may use the literal
// port number in their liveness and readiness probes, and upgrading to a controller version
// with a changed HealthzPort will result in crash loops until users update their probe config.
// Note that there are several stock manifests in this repo that also use the literal port number. If you
// update this value, search for the old port number and update the stock manifests also.
HealthzPort = 10254

// MetricsPort is the default port the manager's metrics service listens on.
// Similar to HealthzPort, it may be used in existing user deployment configurations, and its
// literal value is used in several stock manifests, which must be updated along with this value.
MetricsPort = 10255

// DiagnosticsPort is the default port of the manager's diagnostics service listens on.
DiagnosticsPort = 10256

// KongClientEventRecorderComponentName is a KongClient component name used to identify the events recording component.
KongClientEventRecorderComponentName = "kong-client"

// DefaultClusterDomain is the default cluster domain used by the controller.
// TODO: change this in next major release: https://github.com/Kong/kubernetes-ingress-controller/issues/6756
DefaultClusterDomain = ""
)
8 changes: 5 additions & 3 deletions internal/manager/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/kong/kubernetes-ingress-controller/v3/internal/gatewayapi"
"github.com/kong/kubernetes-ingress-controller/v3/internal/konnect"
"github.com/kong/kubernetes-ingress-controller/v3/internal/konnect/nodes"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/consts"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/featuregates"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/metadata"
"github.com/kong/kubernetes-ingress-controller/v3/internal/manager/telemetry"
Expand Down Expand Up @@ -145,8 +146,8 @@ func Run(
setupLog.Info("Initializing Dataplane Client")
var eventRecorder record.EventRecorder
if c.EmitKubernetesEvents {
setupLog.Info("Emitting Kubernetes events enabled, creating an event recorder for " + KongClientEventRecorderComponentName)
eventRecorder = mgr.GetEventRecorderFor(KongClientEventRecorderComponentName)
setupLog.Info("Emitting Kubernetes events enabled, creating an event recorder for " + consts.KongClientEventRecorderComponentName)
eventRecorder = mgr.GetEventRecorderFor(consts.KongClientEventRecorderComponentName)
} else {
setupLog.Info("Emitting Kubernetes events disabled, discarding all events")
// Create an empty record.FakeRecorder with no Events channel to discard all events.
Expand Down Expand Up @@ -181,7 +182,8 @@ func Run(
referenceIndexers := ctrlref.NewCacheIndexers(setupLog.WithName("reference-indexers"))
cache := store.NewCacheStores()
storer := store.New(cache, c.IngressClassName, logger)
configTranslator, err := translator.NewTranslator(logger, storer, c.KongWorkspace, translatorFeatureFlags, NewSchemaServiceGetter(clientsManager))

configTranslator, err := translator.NewTranslator(logger, storer, c.KongWorkspace, translatorFeatureFlags, NewSchemaServiceGetter(clientsManager), c.ClusterDomain)
if err != nil {
return fmt.Errorf("failed to create translator: %w", err)
}
Expand Down

1 comment on commit fbfdbeb

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Go Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: fbfdbeb Previous: c9d315a Ratio
BenchmarkGetPluginRelations 17285 ns/op 7296 B/op 66 allocs/op 7422 ns/op 7296 B/op 66 allocs/op 2.33
BenchmarkGetPluginRelations - ns/op 17285 ns/op 7422 ns/op 2.33

This comment was automatically generated by workflow using github-action-benchmark.

CC: @Kong/k8s-maintainers

Please sign in to comment.