Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle host and listener collisions for TransportServer resource #1415

Merged
merged 4 commits into from
Mar 1, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/nginx-ingress/main.go
Original file line number Diff line number Diff line change
@@ -637,6 +637,7 @@ func main() {
InternalRoutesEnabled: *enableInternalRoutes,
IsPrometheusEnabled: *enablePrometheusMetrics,
IsLatencyMetricsEnabled: *enableLatencyMetrics,
IsTLSPassthroughEnabled: *enableTLSPassthrough,
}

lbc := k8s.NewLoadBalancerController(lbcInput)
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
# Handling Host Collisions
# Handling Host and Listener Collisions

A host collision occurs when multiple resources configure the same `host`. The Ingress Controller supports two options for handling host collisions:
This document explains how the Ingress Controller handles host and listener collisions among resources.

## Winner Selection Algorithm

If multiple resources contend for the same host/listener, the Ingress Controller will pick the winner based on the `creationTimestamp` of the resources: the oldest resource will win. In case there are more than one oldest resource (their `creationTimestamp` is the same), the Ingress Controller will choose the resource with the lexicographically smallest `uid`.

Note: the `creationTimestamp` and `uid` fields are part of the resource [ObjectMeta](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#objectmeta-v1-meta).

## Host Collisions

A host collision occurs when multiple Ingress, VirtualServer, and TransportServer (configured for TLS Passthrough) resources configure the same `host`. The Ingress Controller supports two options for handling host collisions:
* Choosing the winner so that only one resource handles the host.
* Merging configuration of the conflicting resources.

## Choosing the Winner
### Choosing the Winner

Consider the following two resources:
* `cafe-ingress` Ingress:
@@ -31,11 +41,7 @@ Consider the following two resources:
. . .
```

If a user creates both resources in the cluster, a host collision will occur. As a result, the Ingress Controller will pick the winner using the following algorithm:

> If multiple resources contend for the same host, the Ingress Controller will pick the winner based on the `creationTimestamp` of the resources: the oldest resource will win. In case there are more than one oldest resources (their `creationTimestamp` is the same), the Ingress Controller will choose the resource with the lexicographically smallest `uid`.

> Note: the `creationTimestamp` and `uid` fields are part of the resource [ObjectMeta](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#objectmeta-v1-meta).
If a user creates both resources in the cluster, a host collision will occur. As a result, the Ingress Controller will pick the winner using the [winner selection algorithm](#winner-selection-algorithm).

In our example, if `cafe-virtual-server` was created first, it will win the host `cafe.example.com` and the Ingress Controller will reject `cafe-ingress`. This will be reflected in the events and in the resource's status field:
```
@@ -62,8 +68,56 @@ Events:

Similarly, if `cafe-ingress` was created first, it will win `cafe.example.com` and the Ingress Controller will reject `cafe-virtual-server`.

## Merging Configuration for the Same Host
### Merging Configuration for the Same Host

It is possible to merge configuration for multiple Ingress resources for the same host. One common use case for this approach is distributing resources across multiple namespaces. See the [Cross-namespace Configuration](/nginx-ingress-controller/configuration/ingress-resources/cross-namespace-configuration/) doc for more information.

It is *not* possible to merge the configurations for multiple VirtualServer resources for the same host. However, you can split the VirtualServers into multiple VirtualServerRoute resources, which a single VirtualServer can then reference. See the [corresponding example](https://github.com/nginxinc/kubernetes-ingress/tree/master/examples-of-custom-resources/cross-namespace-configuration) on GitHub.

It is *not* possible to merge configuration for multiple TransportServer resources.

## Listener Collisions

Listener collisions occur when multiple TransportServer resources (configured for TCP/UDP load balancing) configure the same `listener`. The Ingress Controller will choose the winner, which will own the listener.

### Choosing the Winner

Consider the following two resources:
* `tcp-1` TransportServer:
```yaml
apiVersion: k8s.nginx.org/v1alpha1
kind: TransportServer
metadata:
name: tcp-1
spec:
listener:
name: dns-tcp
protocol: TCP
. . .
```
* `tcp-2` TransportServer:
```yaml
apiVersion: k8s.nginx.org/v1alpha1
kind: TransportServer
metadata:
name: tcp-2
spec:
listener:
name: dns-tcp
protocol: TCP
. . .
```

If a user creates both resources in the cluster, a listener collision will occur. As a result, the Ingress Controller will pick the winner using the [winner selection algorithm](#winner-selection-algorithm).

In our example, if `tcp-1` was created first, it will win the listener `dns-tcp` and the Ingress Controller will reject `tcp-2`. This will be reflected in the events and in the resource's status field:
```
$ kubectl describe ts tcp-2
. . .
Events:
Type Reason Age From Message
---- ------ ---- ---- -------
Warning Rejected 10s nginx-ingress-controller Listener dns-tcp is taken by another resource
```

Similarly, if `tcp-2` was created first, it will win `dns-tcp` and the Ingress Controller will reject `tcp-1`.
2 changes: 1 addition & 1 deletion docs-web/configuration/index.rst
Original file line number Diff line number Diff line change
@@ -7,7 +7,7 @@ Configuration
global-configuration/index
ingress-resources/index
virtualserver-and-virtualserverroute-resources
handling-host-collisions
handling-host-and-listener-collisions
policy-resource
transportserver-resource
configuration-examples
6 changes: 2 additions & 4 deletions docs-web/configuration/transportserver-resource.md
Original file line number Diff line number Diff line change
@@ -425,7 +425,5 @@ The [ConfigMap](/nginx-ingress-controller/configuration/global-configuration/con

## Limitations

As of Release 1.7, the TransportServer resource is a preview feature. Currently, it comes with the following limitations:
* When using TLS Passthrough, it is not possible to configure [Proxy Protocol](https://github.com/nginxinc/kubernetes-ingress/tree/master/examples/proxy-protocol) for port 443 both for regular HTTPS and TLS Passthrough traffic.
* If multiple TCP (or UDP) TransportServers reference the same listener, only one of them will receive the traffic. Moreover, until there is only one TransportServer, NGINX will fail to reload. If this happens, the IC will report a warning event with the `AddedOrUpdatedWithError` reason for the resource, which caused the problem, and also report the error in the logs.
* If multiple TLS Passthrough TransportServers have the same hostname, only one of them will receive the traffic. If this happens, the IC will report a warning in the logs like `host "app.example.com" is used by more than one TransportServers`.
The TransportServer resource is a preview feature. Currently, it comes with the following limitation:
* When using TLS Passthrough, it is not possible to configure [Proxy Protocol](https://github.com/nginxinc/kubernetes-ingress/tree/master/examples/proxy-protocol) for port 443 both for regular HTTPS and TLS Passthrough traffic.
91 changes: 37 additions & 54 deletions internal/configs/configurator.go
Original file line number Diff line number Diff line change
@@ -8,7 +8,6 @@ import (
"encoding/pem"
"fmt"
"os"
"sort"
"strings"

"github.com/nginxinc/kubernetes-ingress/internal/k8s/secrets"
@@ -65,6 +64,14 @@ const (
spiffeKeyFileMode = os.FileMode(0600)
)

// ExtendedResources holds all extended configuration resources, for which Configurator configures NGINX.
type ExtendedResources struct {
IngressExes []*IngressEx
MergeableIngresses []*MergeableIngresses
VirtualServerExes []*VirtualServerEx
TransportServerExes []*TransportServerEx
}

type tlsPassthroughPair struct {
Host string
UnixSocket string
@@ -550,8 +557,7 @@ func (cnf *Configurator) AddOrUpdateTransportServer(transportServerEx *Transport
func (cnf *Configurator) addOrUpdateTransportServer(transportServerEx *TransportServerEx) error {
name := getFileNameForTransportServer(transportServerEx.TransportServer)

listener := cnf.globalCfgParams.Listeners[transportServerEx.TransportServer.Spec.Listener.Name]
tsCfg := generateTransportServerConfig(transportServerEx, listener.Port, cnf.isPlus)
tsCfg := generateTransportServerConfig(transportServerEx, transportServerEx.ListenerPort, cnf.isPlus)

content, err := cnf.templateExecutorV2.ExecuteTransportServerTemplate(&tsCfg)
if err != nil {
@@ -590,11 +596,7 @@ func (cnf *Configurator) GetVirtualServerRoutesForVirtualServer(key string) []*c
}

func (cnf *Configurator) updateTLSPassthroughHostsConfig() error {
cfg, duplicatedHosts := generateTLSPassthroughHostsConfig(cnf.tlsPassthroughPairs)

for _, host := range duplicatedHosts {
glog.Warningf("host %s is used by more than one TransportServers", host)
}
cfg := generateTLSPassthroughHostsConfig(cnf.tlsPassthroughPairs)

content, err := cnf.templateExecutorV2.ExecuteTLSPassthroughHostsTemplate(cfg)
if err != nil {
@@ -606,30 +608,14 @@ func (cnf *Configurator) updateTLSPassthroughHostsConfig() error {
return nil
}

func generateTLSPassthroughHostsConfig(tlsPassthroughPairs map[string]tlsPassthroughPair) (*version2.TLSPassthroughHostsConfig, []string) {
var keys []string

for key := range tlsPassthroughPairs {
keys = append(keys, key)
}

// we sort the keys of tlsPassthroughPairs so that we get the same result for the same input
sort.Strings(keys)

func generateTLSPassthroughHostsConfig(tlsPassthroughPairs map[string]tlsPassthroughPair) *version2.TLSPassthroughHostsConfig {
cfg := version2.TLSPassthroughHostsConfig{}
var duplicatedHosts []string

for _, key := range keys {
pair := tlsPassthroughPairs[key]

if _, exists := cfg[pair.Host]; exists {
duplicatedHosts = append(duplicatedHosts, pair.Host)
}

for _, pair := range tlsPassthroughPairs {
cfg[pair.Host] = pair.UnixSocket
}

return &cfg, duplicatedHosts
return &cfg
}

func (cnf *Configurator) addOrUpdateCASecret(secret *api_v1.Secret) string {
@@ -645,33 +631,40 @@ func (cnf *Configurator) addOrUpdateJWKSecret(secret *api_v1.Secret) string {
}

// AddOrUpdateResources adds or updates configuration for resources.
func (cnf *Configurator) AddOrUpdateResources(ingExes []*IngressEx, mergeableIngresses []*MergeableIngresses, virtualServerExes []*VirtualServerEx) (Warnings, error) {
func (cnf *Configurator) AddOrUpdateResources(resources ExtendedResources) (Warnings, error) {
allWarnings := newWarnings()

for _, ingEx := range ingExes {
for _, ingEx := range resources.IngressExes {
warnings, err := cnf.addOrUpdateIngress(ingEx)
if err != nil {
return allWarnings, fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err)
}
allWarnings.Add(warnings)
}

for _, m := range mergeableIngresses {
for _, m := range resources.MergeableIngresses {
warnings, err := cnf.addOrUpdateMergeableIngress(m)
if err != nil {
return allWarnings, fmt.Errorf("Error adding or updating mergeableIngress %v/%v: %v", m.Master.Ingress.Namespace, m.Master.Ingress.Name, err)
}
allWarnings.Add(warnings)
}

for _, vsEx := range virtualServerExes {
for _, vsEx := range resources.VirtualServerExes {
warnings, err := cnf.addOrUpdateVirtualServer(vsEx)
if err != nil {
return allWarnings, fmt.Errorf("Error adding or updating VirtualServer %v/%v: %v", vsEx.VirtualServer.Namespace, vsEx.VirtualServer.Name, err)
}
allWarnings.Add(warnings)
}

for _, tsEx := range resources.TransportServerExes {
err := cnf.addOrUpdateTransportServer(tsEx)
if err != nil {
return allWarnings, fmt.Errorf("Error adding or updating TransportServer %v/%v: %v", tsEx.TransportServer.Namespace, tsEx.TransportServer.Name, err)
}
}

if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil {
return allWarnings, fmt.Errorf("Error when reloading NGINX when updating resources: %v", err)
}
@@ -1080,36 +1073,26 @@ func (cnf *Configurator) UpdateConfig(cfgParams *ConfigParams, ingExes []*Ingres
return allWarnings, nil
}

// UpdateGlobalConfiguration updates NGINX config based on the changes to the GlobalConfiguration resource.
// Currently, changes to the GlobalConfiguration only affect TransportServer resources.
// As a result of the changes, the configuration for TransportServers is updated and some TransportServers
// might be removed from NGINX.
func (cnf *Configurator) UpdateGlobalConfiguration(globalConfiguration *conf_v1alpha1.GlobalConfiguration,
transportServerExes []*TransportServerEx) (updatedTransportServerExes []*TransportServerEx, deletedTransportServerExes []*TransportServerEx, err error) {
cnf.globalCfgParams = ParseGlobalConfiguration(globalConfiguration, cnf.staticCfgParams.TLSPassthrough)

for _, tsEx := range transportServerExes {
if cnf.CheckIfListenerExists(&tsEx.TransportServer.Spec.Listener) {
updatedTransportServerExes = append(updatedTransportServerExes, tsEx)

err := cnf.addOrUpdateTransportServer(tsEx)
if err != nil {
return updatedTransportServerExes, deletedTransportServerExes, fmt.Errorf("Error when updating global configuration: %v", err)
}
func (cnf *Configurator) UpdateTransportServers(updatedTSExes []*TransportServerEx, deletedKeys []string) error {
for _, tsEx := range updatedTSExes {
err := cnf.addOrUpdateTransportServer(tsEx)
if err != nil {
return fmt.Errorf("Error adding or updating TransportServer %v/%v: %v", tsEx.TransportServer.Namespace, tsEx.TransportServer.Name, err)
}
}

} else {
deletedTransportServerExes = append(deletedTransportServerExes, tsEx)
if err != nil {
return updatedTransportServerExes, deletedTransportServerExes, fmt.Errorf("Error when updating global configuration: %v", err)
}
for _, key := range deletedKeys {
err := cnf.deleteTransportServer(key)
if err != nil {
return fmt.Errorf("Error when removing TransportServer %v: %v", key, err)
}
}

if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil {
return updatedTransportServerExes, deletedTransportServerExes, fmt.Errorf("Error when updating global configuration: %v", err)
return fmt.Errorf("Error when updating TransportServers: %v", err)
}

return updatedTransportServerExes, deletedTransportServerExes, nil
return nil
}

func keyToFileName(key string) string {
105 changes: 5 additions & 100 deletions internal/configs/configurator_test.go
Original file line number Diff line number Diff line change
@@ -327,122 +327,27 @@ func TestGenerateNamespaceNameKey(t *testing.T) {
}
}

func TestUpdateGlobalConfiguration(t *testing.T) {
globalConfiguration := &conf_v1alpha1.GlobalConfiguration{
Spec: conf_v1alpha1.GlobalConfigurationSpec{
Listeners: []conf_v1alpha1.Listener{
{
Name: "tcp-listener",
Port: 53,
Protocol: "TCP",
},
},
},
}

tsExTCP := &TransportServerEx{
TransportServer: &conf_v1alpha1.TransportServer{
ObjectMeta: meta_v1.ObjectMeta{
Name: "tcp-server",
Namespace: "default",
},
Spec: conf_v1alpha1.TransportServerSpec{
Listener: conf_v1alpha1.TransportServerListener{
Name: "tcp-listener",
Protocol: "TCP",
},
Upstreams: []conf_v1alpha1.Upstream{
{
Name: "tcp-app",
Service: "tcp-app-svc",
Port: 5001,
},
},
Action: &conf_v1alpha1.Action{
Pass: "tcp-app",
},
},
},
}

tsExUDP := &TransportServerEx{
TransportServer: &conf_v1alpha1.TransportServer{
ObjectMeta: meta_v1.ObjectMeta{
Name: "udp-server",
Namespace: "default",
},
Spec: conf_v1alpha1.TransportServerSpec{
Listener: conf_v1alpha1.TransportServerListener{
Name: "udp-listener",
Protocol: "UDP",
},
Upstreams: []conf_v1alpha1.Upstream{
{
Name: "udp-app",
Service: "udp-app-svc",
Port: 5001,
},
},
Action: &conf_v1alpha1.Action{
Pass: "udp-app",
},
},
},
}

cnf, err := createTestConfigurator()
if err != nil {
t.Fatalf("Failed to create a test configurator: %v", err)
}

transportServerExes := []*TransportServerEx{tsExTCP, tsExUDP}

expectedUpdatedTransportServerExes := []*TransportServerEx{tsExTCP}
expectedDeletedTransportServerExes := []*TransportServerEx{tsExUDP}

updatedTransportServerExes, deletedTransportServerExes, err := cnf.UpdateGlobalConfiguration(globalConfiguration, transportServerExes)

if !reflect.DeepEqual(updatedTransportServerExes, expectedUpdatedTransportServerExes) {
t.Errorf("UpdateGlobalConfiguration() returned %v but expected %v", updatedTransportServerExes, expectedUpdatedTransportServerExes)
}
if !reflect.DeepEqual(deletedTransportServerExes, expectedDeletedTransportServerExes) {
t.Errorf("UpdateGlobalConfiguration() returned %v but expected %v", deletedTransportServerExes, expectedDeletedTransportServerExes)
}
if err != nil {
t.Errorf("UpdateGlobalConfiguration() returned an unexpected error %v", err)
}
}

func TestGenerateTLSPassthroughHostsConfig(t *testing.T) {
tlsPassthroughPairs := map[string]tlsPassthroughPair{
"default/ts-1": {
Host: "app.example.com",
Host: "one.example.com",
UnixSocket: "socket1.sock",
},
"default/ts-2": {
Host: "app.example.com",
Host: "two.example.com",
UnixSocket: "socket2.sock",
},
"default/ts-3": {
Host: "some.example.com",
UnixSocket: "socket3.sock",
},
}

expectedCfg := &version2.TLSPassthroughHostsConfig{
"app.example.com": "socket2.sock",
"some.example.com": "socket3.sock",
"one.example.com": "socket1.sock",
"two.example.com": "socket2.sock",
}
expectedDuplicatedHosts := []string{"app.example.com"}

resultCfg, resultDuplicatedHosts := generateTLSPassthroughHostsConfig(tlsPassthroughPairs)
resultCfg := generateTLSPassthroughHostsConfig(tlsPassthroughPairs)
if !reflect.DeepEqual(resultCfg, expectedCfg) {
t.Errorf("generateTLSPassthroughHostsConfig() returned %v but expected %v", resultCfg, expectedCfg)
}

if !reflect.DeepEqual(resultDuplicatedHosts, expectedDuplicatedHosts) {
t.Errorf("generateTLSPassthroughHostsConfig() returned %v but expected %v", resultDuplicatedHosts, expectedDuplicatedHosts)
}
}

func TestAddInternalRouteConfig(t *testing.T) {
1 change: 1 addition & 0 deletions internal/configs/transportserver.go
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@ const nginxNonExistingUnixSocket = "unix:/var/lib/nginx/non-existing-unix-socket

// TransportServerEx holds a TransportServer along with the resources referenced by it.
type TransportServerEx struct {
ListenerPort int
TransportServer *conf_v1alpha1.TransportServer
Endpoints map[string][]string
PodsByIP map[string]string
571 changes: 493 additions & 78 deletions internal/k8s/configuration.go

Large diffs are not rendered by default.

1,043 changes: 964 additions & 79 deletions internal/k8s/configuration_test.go

Large diffs are not rendered by default.

431 changes: 202 additions & 229 deletions internal/k8s/controller.go

Large diffs are not rendered by default.

58 changes: 0 additions & 58 deletions internal/k8s/controller_test.go
Original file line number Diff line number Diff line change
@@ -17,7 +17,6 @@ import (
"github.com/nginxinc/kubernetes-ingress/internal/metrics/collectors"
"github.com/nginxinc/kubernetes-ingress/internal/nginx"
conf_v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1"
conf_v1alpha1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1alpha1"
api_v1 "k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
networking "k8s.io/api/networking/v1beta1"
@@ -569,63 +568,6 @@ func TestGetServicePortForIngressPort(t *testing.T) {
}
}

func TestFindTransportServersForService(t *testing.T) {
ts1 := conf_v1alpha1.TransportServer{
ObjectMeta: meta_v1.ObjectMeta{
Name: "ts-1",
Namespace: "ns-1",
},
Spec: conf_v1alpha1.TransportServerSpec{
Upstreams: []conf_v1alpha1.Upstream{
{
Service: "test-service",
},
},
},
}
ts2 := conf_v1alpha1.TransportServer{
ObjectMeta: meta_v1.ObjectMeta{
Name: "ts-2",
Namespace: "ns-1",
},
Spec: conf_v1alpha1.TransportServerSpec{
Upstreams: []conf_v1alpha1.Upstream{
{
Service: "some-service",
},
},
},
}
ts3 := conf_v1alpha1.TransportServer{
ObjectMeta: meta_v1.ObjectMeta{
Name: "ts-3",
Namespace: "ns-2",
},
Spec: conf_v1alpha1.TransportServerSpec{
Upstreams: []conf_v1alpha1.Upstream{
{
Service: "test-service",
},
},
},
}
transportServers := []*conf_v1alpha1.TransportServer{&ts1, &ts2, &ts3}

service := v1.Service{
ObjectMeta: meta_v1.ObjectMeta{
Name: "test-service",
Namespace: "ns-1",
},
}

expected := []*conf_v1alpha1.TransportServer{&ts1}

result := findTransportServersForService(transportServers, &service)
if !reflect.DeepEqual(result, expected) {
t.Errorf("findTransportServersForService returned %v but expected %v", result, expected)
}
}

func TestFormatWarningsMessages(t *testing.T) {
warnings := []string{"Test warning", "Test warning 2"}

14 changes: 0 additions & 14 deletions internal/k8s/handlers.go
Original file line number Diff line number Diff line change
@@ -188,19 +188,13 @@ func createSecretHandlers(lbc *LoadBalancerController) cache.ResourceEventHandle
// or a change of the externalName field of an ExternalName service.
//
// In both cases we enqueue the service to be processed by syncService
// Also, because TransportServers aren't processed by syncService,
// we enqueue them, so they get processed by syncTransportServer.
func createServiceHandlers(lbc *LoadBalancerController) cache.ResourceEventHandlerFuncs {
return cache.ResourceEventHandlerFuncs{
AddFunc: func(obj interface{}) {
svc := obj.(*v1.Service)

glog.V(3).Infof("Adding service: %v", svc.Name)
lbc.AddSyncQueue(svc)

if lbc.areCustomResourcesEnabled {
lbc.EnqueueTransportServerForService(svc)
}
},
DeleteFunc: func(obj interface{}) {
svc, isSvc := obj.(*v1.Service)
@@ -219,10 +213,6 @@ func createServiceHandlers(lbc *LoadBalancerController) cache.ResourceEventHandl

glog.V(3).Infof("Removing service: %v", svc.Name)
lbc.AddSyncQueue(svc)

if lbc.areCustomResourcesEnabled {
lbc.EnqueueTransportServerForService(svc)
}
},
UpdateFunc: func(old, cur interface{}) {
if !reflect.DeepEqual(old, cur) {
@@ -235,10 +225,6 @@ func createServiceHandlers(lbc *LoadBalancerController) cache.ResourceEventHandl
if hasServiceChanges(oldSvc, curSvc) {
glog.V(3).Infof("Service %v changed, syncing", curSvc.Name)
lbc.AddSyncQueue(curSvc)

if lbc.areCustomResourcesEnabled {
lbc.EnqueueTransportServerForService(curSvc)
}
}
}
},
28 changes: 28 additions & 0 deletions internal/k8s/reference_checkers.go
Original file line number Diff line number Diff line change
@@ -3,6 +3,7 @@ package k8s
import (
"github.com/nginxinc/kubernetes-ingress/internal/configs"
v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1"
conf_v1alpha1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1alpha1"
networking "k8s.io/api/networking/v1beta1"
)

@@ -11,6 +12,7 @@ type resourceReferenceChecker interface {
IsReferencedByMinion(namespace string, name string, ing *networking.Ingress) bool
IsReferencedByVirtualServer(namespace string, name string, vs *v1.VirtualServer) bool
IsReferencedByVirtualServerRoute(namespace string, name string, vsr *v1.VirtualServerRoute) bool
IsReferencedByTransportServer(namespace string, name string, ts *conf_v1alpha1.TransportServer) bool
}

type secretReferenceChecker struct {
@@ -75,6 +77,10 @@ func (rc *secretReferenceChecker) IsReferencedByVirtualServerRoute(secretNamespa
return false
}

func (rc *secretReferenceChecker) IsReferencedByTransportServer(secretNamespace string, secretName string, ts *conf_v1alpha1.TransportServer) bool {
return false
}

type serviceReferenceChecker struct{}

func newServiceReferenceChecker() *serviceReferenceChecker {
@@ -137,6 +143,20 @@ func (rc *serviceReferenceChecker) IsReferencedByVirtualServerRoute(svcNamespace
return false
}

func (rc *serviceReferenceChecker) IsReferencedByTransportServer(svcNamespace string, svcName string, ts *conf_v1alpha1.TransportServer) bool {
if ts.Namespace != svcNamespace {
return false
}

for _, u := range ts.Spec.Upstreams {
if u.Service == svcName {
return true
}
}

return false
}

type policyReferenceChecker struct {
}

@@ -176,6 +196,10 @@ func (rc *policyReferenceChecker) IsReferencedByVirtualServerRoute(policyNamespa
return false
}

func (rc *policyReferenceChecker) IsReferencedByTransportServer(policyNamespace string, policyName string, ts *conf_v1alpha1.TransportServer) bool {
return false
}

// appProtectResourceReferenceChecker is a reference checker for AppProtect related resources.
// Only Regular/Master Ingress can reference those resources.
type appProtectResourceReferenceChecker struct {
@@ -207,6 +231,10 @@ func (rc *appProtectResourceReferenceChecker) IsReferencedByVirtualServerRoute(n
return false
}

func (rc *appProtectResourceReferenceChecker) IsReferencedByTransportServer(namespace string, name string, ts *conf_v1alpha1.TransportServer) bool {
return false
}

func isPolicyReferenced(policies []v1.PolicyReference, resourceNamespace string, policyNamespace string, policyName string) bool {
for _, p := range policies {
namespace := p.Namespace
104 changes: 97 additions & 7 deletions internal/k8s/reference_checkers_test.go
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@ import (

"github.com/nginxinc/kubernetes-ingress/internal/configs"
conf_v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1"
conf_v1alpha1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1alpha1"
networking "k8s.io/api/networking/v1beta1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
@@ -301,13 +302,23 @@ func TestSecretIsReferencedByVirtualServerRoute(t *testing.T) {
isPlus := false // doesn't matter for VirtualServerRoute
rc := newSecretReferenceChecker(isPlus)

// always returns false
result := rc.IsReferencedByVirtualServerRoute("", "", nil)
if result != false {
t.Error("IsReferencedByVirtualServer() returned true but expected false")
}
}

func TestSecretIsReferencedByTransportServer(t *testing.T) {
isPlus := false // doesn't matter for TransportServer
rc := newSecretReferenceChecker(isPlus)

// always returns false
result := rc.IsReferencedByTransportServer("", "", nil)
if result != false {
t.Error("IsReferencedByTransportServer() returned true but expected false")
}
}

func TestServiceIsReferencedByIngressAndMinion(t *testing.T) {
tests := []struct {
ing *networking.Ingress
@@ -550,20 +561,97 @@ func TestServiceIsReferencedByVirtualServerAndVirtualServerRoutes(t *testing.T)
}
}

func TestPolicyIsReferencedByIngresses(t *testing.T) {
func TestIsServiceReferencedByTransportServer(t *testing.T) {
tests := []struct {
ts *conf_v1alpha1.TransportServer
serviceNamespace string
serviceName string
expected bool
msg string
}{
{
ts: &conf_v1alpha1.TransportServer{
ObjectMeta: v1.ObjectMeta{
Namespace: "default",
},
Spec: conf_v1alpha1.TransportServerSpec{
Upstreams: []conf_v1alpha1.Upstream{
{
Service: "test-service",
},
},
},
},
serviceNamespace: "default",
serviceName: "test-service",
expected: true,
msg: "service is referenced in an upstream",
},
{
ts: &conf_v1alpha1.TransportServer{
ObjectMeta: v1.ObjectMeta{
Namespace: "default",
},
Spec: conf_v1alpha1.TransportServerSpec{
Upstreams: []conf_v1alpha1.Upstream{
{
Service: "test-service",
},
},
},
},
serviceNamespace: "some-namespace",
serviceName: "test-service",
expected: false,
msg: "wrong namespace for service in an upstream",
},
{
ts: &conf_v1alpha1.TransportServer{
ObjectMeta: v1.ObjectMeta{
Namespace: "default",
},
Spec: conf_v1alpha1.TransportServerSpec{
Upstreams: []conf_v1alpha1.Upstream{
{
Service: "test-service",
},
},
},
},
serviceNamespace: "default",
serviceName: "some-service",
expected: false,
msg: "wrong name for service in an upstream",
},
}

for _, test := range tests {
rc := newServiceReferenceChecker()

result := rc.IsReferencedByTransportServer(test.serviceNamespace, test.serviceName, test.ts)
if result != test.expected {
t.Errorf("IsReferencedByTransportServer() returned %v but expected %v for the case of %s", result, test.expected, test.msg)
}
}
}

func TestPolicyIsReferencedByIngressesAndTransportServers(t *testing.T) {
rc := newPolicyReferenceChecker()

// always returns false
result := rc.IsReferencedByIngress("", "", nil)
if result != false {
t.Error("IsReferencedByIngress() returned true but expected false")
}

// always returns false
result = rc.IsReferencedByMinion("", "", nil)
if result != false {
t.Error("IsReferencedByMinion() returned true but expected false")
}

result = rc.IsReferencedByTransportServer("", "", nil)
if result != false {
t.Error("IsReferencedByTransportServer() returned true but expected false")
}
}

func TestPolicyIsReferencedByVirtualServerAndVirtualServerRoute(t *testing.T) {
@@ -861,7 +949,6 @@ func TestAppProtectResourceIsReferencedByIngresses(t *testing.T) {
t.Errorf("IsReferencedByIngress() returned %v but expected %v for the case of %s", result, test.expected, test.msg)
}

// always false for minion
result = rc.IsReferencedByMinion(test.resourceNamespace, test.resourceName, test.ing)
if result != false {
t.Errorf("IsReferencedByMinion() returned true but expected false for the case of %s", test.msg)
@@ -872,17 +959,20 @@ func TestAppProtectResourceIsReferencedByIngresses(t *testing.T) {
func TestAppProtectResourceIsReferenced(t *testing.T) {
rc := newAppProtectResourceReferenceChecker("test")

// always returns false
result := rc.IsReferencedByVirtualServer("", "", nil)
if result != false {
t.Error("IsReferencedByVirtualServer() returned true but expected false")
}

// always returns false
result = rc.IsReferencedByVirtualServerRoute("", "", nil)
if result != false {
t.Error("IsReferencedByVirtualServer() returned true but expected false")
}

result = rc.IsReferencedByTransportServer("", "", nil)
if result != false {
t.Error("IsReferencedByTransportServer() returned true but expected false")
}
}

func TestIsPolicyIsReferenced(t *testing.T) {
2 changes: 1 addition & 1 deletion pkg/apis/configuration/validation/transportserver.go
Original file line number Diff line number Diff line change
@@ -40,7 +40,7 @@ func (tsv *TransportServerValidator) validateTransportServerSpec(spec *v1alpha1.

allErrs = append(allErrs, validateTransportServerUpstreamParameters(spec.UpstreamParameters, fieldPath.Child("upstreamParameters"), spec.Listener.Protocol)...)

allErrs = append(validateSessionParameters(spec.SessionParameters, fieldPath.Child("sessionParameters")))
allErrs = append(allErrs, validateSessionParameters(spec.SessionParameters, fieldPath.Child("sessionParameters"))...)

if spec.Action == nil {
allErrs = append(allErrs, field.Required(fieldPath.Child("action"), "must specify action"))