Skip to content

Commit

Permalink
feat(#3097): propagate translation failures for ingress referred secr…
Browse files Browse the repository at this point in the history 
…ets (#3150)
  • Loading branch information
@czeslavo
czeslavo committed Nov 15, 2022
1 parent 63359c7 commit 158c3a6
Show file tree
Hide file tree
Showing 11 changed files with 211 additions and 81 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,9 @@ Adding a new version? You'll need three changes:
- Warning events are recorded when a Gateway Listener has more than one CertificateRef specified
or refers to a Secret that has no valid TLS key-pair.
[#3147](https://github.com/Kong/kubernetes-ingress-controller/pull/3147)
- Warning events are recorded when an Ingress refers to a TLS secret that does not exist or
has no valid TLS key-pair.
[#3150](https://github.com/Kong/kubernetes-ingress-controller/pull/3150)

### Fixed

Expand Down
87 changes: 62 additions & 25 deletions internal/dataplane/parser/ingressrules.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
corev1 "k8s.io/api/core/v1"
netv1 "k8s.io/api/networking/v1"
netv1beta1 "k8s.io/api/networking/v1beta1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/kong/kubernetes-ingress-controller/v2/internal/annotations"
"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate"
Expand All @@ -32,7 +34,11 @@ func mergeIngressRules(objs ...ingressRules) ingressRules {

for _, obj := range objs {
for k, v := range obj.SecretNameToSNIs {
result.SecretNameToSNIs[k] = append(result.SecretNameToSNIs[k], v...)
if _, ok := result.SecretNameToSNIs[k]; !ok {
result.SecretNameToSNIs[k] = &SNIs{}
}
result.SecretNameToSNIs[k].hosts = append(result.SecretNameToSNIs[k].hosts, v.hosts...)
result.SecretNameToSNIs[k].parents = append(result.SecretNameToSNIs[k].parents, v.parents...)
}
for k, v := range obj.ServiceNameToServices {
result.ServiceNameToServices[k] = v
Expand Down Expand Up @@ -84,7 +90,10 @@ func (ir *ingressRules) populateServices(log logrus.FieldLogger, s store.Storer,

// ensure that the cert is loaded into Kong
if _, ok := ir.SecretNameToSNIs[secretKey]; !ok {
ir.SecretNameToSNIs[secretKey] = []string{}
ir.SecretNameToSNIs[secretKey] = &SNIs{
parents: []client.Object{k8sService},
hosts: []string{},
}
}
service.ClientCertificate = &kong.Certificate{
ID: kong.String(string(secret.UID)),
Expand All @@ -99,53 +108,73 @@ func (ir *ingressRules) populateServices(log logrus.FieldLogger, s store.Storer,
return serviceNamesToSkip
}

type SecretNameToSNIs map[string][]string
type SecretNameToSNIs map[string]*SNIs

func newSecretNameToSNIs() SecretNameToSNIs {
return SecretNameToSNIs(map[string][]string{})
type SNIs struct {
parents []client.Object
hosts []string
}

func (m SecretNameToSNIs) addFromIngressV1beta1TLS(tlsSections []netv1beta1.IngressTLS, namespace string) {
// Assume that v1beta1 and v1 tlsSections have identical semantics and field-wise content.
var v1 []netv1.IngressTLS
for _, item := range tlsSections {
v1 = append(v1, netv1.IngressTLS{Hosts: item.Hosts, SecretName: item.SecretName})
}
m.addFromIngressV1TLS(v1, namespace)
func newSecretNameToSNIs() SecretNameToSNIs {
return map[string]*SNIs{}
}

func (m SecretNameToSNIs) addFromIngressV1TLS(tlsSections []netv1.IngressTLS, namespace string) {
func (m SecretNameToSNIs) addFromIngressV1TLS(tlsSections []netv1.IngressTLS, parent client.Object) {
for _, tls := range tlsSections {
if len(tls.Hosts) == 0 {
continue
}
if tls.SecretName == "" {
continue
}
hosts := tls.Hosts
secretName := namespace + "/" + tls.SecretName
hosts = m.filterHosts(hosts)
if m[secretName] != nil {
hosts = append(hosts, m[secretName]...)
}
m[secretName] = hosts

secretKey := parent.GetNamespace() + "/" + tls.SecretName
m.addUniqueHosts(secretKey, tls.Hosts)
m.addUniqueParents(secretKey, []client.Object{parent})
}
}

func (m SecretNameToSNIs) filterHosts(hosts []string) []string {
hostsToAdd := []string{}
func (m SecretNameToSNIs) addUniqueHosts(secretKey string, hosts []string) {
if _, ok := m[secretKey]; !ok {
m[secretKey] = &SNIs{}
}

seenHosts := map[string]bool{}
for _, hosts := range m {
for _, host := range hosts {
for _, snis := range m {
for _, host := range snis.hosts {
seenHosts[host] = true
}
}

var hostsToAdd []string
for _, host := range hosts {
if !seenHosts[host] {
hostsToAdd = append(hostsToAdd, host)
}
}
return hostsToAdd

m[secretKey].hosts = append(m[secretKey].hosts, hostsToAdd...)
}

// TODO: https://github.com/Kong/kubernetes-ingress-controller/issues/3166
func (m SecretNameToSNIs) addUniqueParents(secretKey string, parents []client.Object) {
if _, ok := m[secretKey]; !ok {
m[secretKey] = &SNIs{}
}

seenParents := map[types.UID]struct{}{}
for _, parent := range m[secretKey].parents {
seenParents[parent.GetUID()] = struct{}{}
}

var parentsToAdd []client.Object
for _, parent := range parents {
if _, ok := seenParents[parent.GetUID()]; !ok {
parentsToAdd = append(parentsToAdd, parent)
}
}

m[secretKey].parents = append(m[secretKey].parents, parentsToAdd...)
}

func getK8sServicesForBackends(
Expand Down Expand Up @@ -233,3 +262,11 @@ func servicesAllUseTheSameKongAnnotations(

return match
}

func v1beta1toV1TLS(tlsSections []netv1beta1.IngressTLS) []netv1.IngressTLS {
var v1 []netv1.IngressTLS
for _, item := range tlsSections {
v1 = append(v1, netv1.IngressTLS{Hosts: item.Hosts, SecretName: item.SecretName})
}
return v1
}
75 changes: 56 additions & 19 deletions internal/dataplane/parser/ingressrules_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,23 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
netv1 "k8s.io/api/networking/v1"
netv1beta1 "k8s.io/api/networking/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/uuid"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate"
"github.com/kong/kubernetes-ingress-controller/v2/internal/store"
)

func TestMergeIngressRules(t *testing.T) {
var (
parent1 = &netv1.Ingress{ObjectMeta: metav1.ObjectMeta{UID: uuid.NewUUID()}}
parent2 = &netv1.Ingress{ObjectMeta: metav1.ObjectMeta{UID: uuid.NewUUID()}}
)

for _, tt := range []struct {
name string
inputs []ingressRules
Expand All @@ -27,7 +35,7 @@ func TestMergeIngressRules(t *testing.T) {
{
name: "empty list",
wantOutput: &ingressRules{
SecretNameToSNIs: map[string][]string{},
SecretNameToSNIs: map[string]*SNIs{},
ServiceNameToServices: map[string]kongstate.Service{},
},
},
Expand All @@ -37,54 +45,84 @@ func TestMergeIngressRules(t *testing.T) {
{}, {}, {},
},
wantOutput: &ingressRules{
SecretNameToSNIs: map[string][]string{},
SecretNameToSNIs: map[string]*SNIs{},
ServiceNameToServices: map[string]kongstate.Service{},
},
},
{
name: "one input",
inputs: []ingressRules{
{
SecretNameToSNIs: map[string][]string{"a": {"b", "c"}, "d": {"e", "f"}},
SecretNameToSNIs: map[string]*SNIs{"a": {hosts: []string{"b", "c"}}, "d": {hosts: []string{"e", "f"}}},
ServiceNameToServices: map[string]kongstate.Service{"1": {Namespace: "potato"}},
},
},
wantOutput: &ingressRules{
SecretNameToSNIs: map[string][]string{"a": {"b", "c"}, "d": {"e", "f"}},
SecretNameToSNIs: map[string]*SNIs{"a": {hosts: []string{"b", "c"}}, "d": {hosts: []string{"e", "f"}}},
ServiceNameToServices: map[string]kongstate.Service{"1": {Namespace: "potato"}},
},
},
{
name: "three inputs",
inputs: []ingressRules{
{
SecretNameToSNIs: map[string][]string{"a": {"b", "c"}, "d": {"e", "f"}},
SecretNameToSNIs: map[string]*SNIs{"a": {hosts: []string{"b", "c"}}, "d": {hosts: []string{"e", "f"}}},
ServiceNameToServices: map[string]kongstate.Service{"1": {Namespace: "potato"}},
},
{
SecretNameToSNIs: map[string][]string{"g": {"h"}},
SecretNameToSNIs: map[string]*SNIs{"g": {hosts: []string{"h"}}},
},
{
ServiceNameToServices: map[string]kongstate.Service{"2": {Namespace: "carrot"}},
},
},
wantOutput: &ingressRules{
SecretNameToSNIs: map[string][]string{"a": {"b", "c"}, "d": {"e", "f"}, "g": {"h"}},
SecretNameToSNIs: map[string]*SNIs{"a": {hosts: []string{"b", "c"}}, "d": {hosts: []string{"e", "f"}}, "g": {hosts: []string{"h"}}},
ServiceNameToServices: map[string]kongstate.Service{"1": {Namespace: "potato"}, "2": {Namespace: "carrot"}},
},
},
{
name: "can merge SNI arrays",
inputs: []ingressRules{
{
SecretNameToSNIs: map[string][]string{"a": {"b", "c"}},
SecretNameToSNIs: map[string]*SNIs{"a": {hosts: []string{"b", "c"}}},
},
{
SecretNameToSNIs: map[string]*SNIs{"a": {hosts: []string{"d", "e"}}},
},
},
wantOutput: &ingressRules{
SecretNameToSNIs: map[string]*SNIs{"a": {hosts: []string{"b", "c", "d", "e"}}},
ServiceNameToServices: map[string]kongstate.Service{},
},
},
{
name: "can merge SNI arrays with parents",
inputs: []ingressRules{
{
SecretNameToSNIs: map[string]*SNIs{"a": {parents: []client.Object{parent1}, hosts: []string{"b", "c"}}},
},
{
SecretNameToSNIs: map[string][]string{"a": {"d", "e"}},
SecretNameToSNIs: map[string]*SNIs{"a": {parents: []client.Object{parent2}, hosts: []string{"d", "e"}}},
},
},
wantOutput: &ingressRules{
SecretNameToSNIs: map[string][]string{"a": {"b", "c", "d", "e"}},
SecretNameToSNIs: map[string]*SNIs{"a": {parents: []client.Object{parent1, parent2}, hosts: []string{"b", "c", "d", "e"}}},
ServiceNameToServices: map[string]kongstate.Service{},
},
},
{
name: "can merge SNI arrays with repeating parents",
inputs: []ingressRules{
{
SecretNameToSNIs: map[string]*SNIs{"a": {parents: []client.Object{parent1, parent2}, hosts: []string{"b", "c"}}},
},
{
SecretNameToSNIs: map[string]*SNIs{"a": {parents: []client.Object{parent1, parent2}, hosts: []string{"d", "e"}}},
},
},
wantOutput: &ingressRules{
SecretNameToSNIs: map[string]*SNIs{"a": {parents: []client.Object{parent1, parent2, parent1, parent2}, hosts: []string{"b", "c", "d", "e"}}},
ServiceNameToServices: map[string]kongstate.Service{},
},
},
Expand All @@ -99,7 +137,7 @@ func TestMergeIngressRules(t *testing.T) {
},
},
wantOutput: &ingressRules{
SecretNameToSNIs: map[string][]string{},
SecretNameToSNIs: newSecretNameToSNIs(),
ServiceNameToServices: map[string]kongstate.Service{"svc-name": {Namespace: "new"}},
},
},
Expand All @@ -112,9 +150,10 @@ func TestMergeIngressRules(t *testing.T) {
}

func TestAddFromIngressV1beta1TLS(t *testing.T) {
parentIngress := &netv1beta1.Ingress{ObjectMeta: metav1.ObjectMeta{Namespace: "foo"}}

type args struct {
tlsSections []netv1beta1.IngressTLS
namespace string
}
tests := []struct {
name string
Expand All @@ -139,11 +178,10 @@ func TestAddFromIngressV1beta1TLS(t *testing.T) {
SecretName: "sooper-secret2",
},
},
namespace: "foo",
},
want: SecretNameToSNIs{
"foo/sooper-secret": {"1.example.com", "2.example.com"},
"foo/sooper-secret2": {"3.example.com", "4.example.com"},
"foo/sooper-secret": {hosts: []string{"1.example.com", "2.example.com"}, parents: []client.Object{parentIngress}},
"foo/sooper-secret2": {hosts: []string{"3.example.com", "4.example.com"}, parents: []client.Object{parentIngress}},
},
},
{
Expand All @@ -164,18 +202,17 @@ func TestAddFromIngressV1beta1TLS(t *testing.T) {
SecretName: "sooper-secret2",
},
},
namespace: "foo",
},
want: SecretNameToSNIs{
"foo/sooper-secret": {"1.example.com"},
"foo/sooper-secret2": {"3.example.com", "4.example.com"},
"foo/sooper-secret": {hosts: []string{"1.example.com"}, parents: []client.Object{parentIngress}},
"foo/sooper-secret2": {hosts: []string{"3.example.com", "4.example.com"}, parents: []client.Object{parentIngress}},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
m := newSecretNameToSNIs()
m.addFromIngressV1beta1TLS(tt.args.tlsSections, tt.args.namespace)
m.addFromIngressV1TLS(v1beta1toV1TLS(tt.args.tlsSections), parentIngress)
assert.Equal(t, m, tt.want)
})
}
Expand Down
Loading

0 comments on commit 158c3a6

Please sign in to comment.