-
Notifications
You must be signed in to change notification settings - Fork 28
make Gateway controllers watch changes on Secrets referenced by spec.listeners.tls.certificateRef #2661
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
base: main
Are you sure you want to change the base?
make Gateway controllers watch changes on Secrets referenced by spec.listeners.tls.certificateRef #2661
Changes from all commits
328af64
75f3965
49b2f41
d54c7aa
eb39c63
b831014
50837e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| package index | ||
|
|
||
| import ( | ||
| corev1 "k8s.io/api/core/v1" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
|
|
||
| gwtypes "github.com/kong/kong-operator/internal/types" | ||
| ) | ||
|
|
||
| const ( | ||
| // TLSCertificateSecretsOnGatewayIndex maps Secret "namespace/name" to Gateway objects that | ||
| // reference them in listeners.tls.certificateRefs. | ||
| TLSCertificateSecretsOnGatewayIndex = "TLSCertificateSecretsOnGateway" | ||
| ) | ||
|
|
||
| // OptionsForGatewayTLSSecret returns index options for Gateways referencing Secrets via TLS certificateRefs. | ||
| func OptionsForGatewayTLSSecret() []Option { | ||
| return []Option{ | ||
| { | ||
| Object: &gwtypes.Gateway{}, | ||
| Field: TLSCertificateSecretsOnGatewayIndex, | ||
| ExtractValueFn: tlsCertificateSecretsOnGateway, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| // tlsCertificateSecretsOnGateway extracts Secret references (namespace/name) from a Gateway's listeners.tls.certificateRefs. | ||
| func tlsCertificateSecretsOnGateway(o client.Object) []string { | ||
| gw, ok := o.(*gwtypes.Gateway) | ||
| if !ok { | ||
| return nil | ||
| } | ||
|
|
||
| seen := make(map[string]struct{}) | ||
| for _, l := range gw.Spec.Listeners { | ||
| if l.TLS == nil { | ||
| continue | ||
| } | ||
| for _, ref := range l.TLS.CertificateRefs { | ||
| // Only index core/v1 Secret references (or defaulted ones when group/kind are nil). | ||
| if ref.Group != nil && string(*ref.Group) != corev1.GroupName { | ||
| continue | ||
| } | ||
| if ref.Kind != nil && string(*ref.Kind) != "Secret" { | ||
| continue | ||
| } | ||
| ns := gw.Namespace | ||
| if ref.Namespace != nil { | ||
| ns = string(*ref.Namespace) | ||
| } | ||
| if ref.Name == "" { | ||
| continue | ||
| } | ||
| seen[ns+"/"+string(ref.Name)] = struct{}{} | ||
| } | ||
| } | ||
|
|
||
| out := make([]string, 0, len(seen)) | ||
| for key := range seen { | ||
| out = append(out, key) | ||
| } | ||
| return out | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,155 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| package envtest | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| import ( | ||||||||||||||||||||||||||||||||||||||||||||||
| "context" | ||||||||||||||||||||||||||||||||||||||||||||||
| "testing" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/samber/lo" | ||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/stretchr/testify/require" | ||||||||||||||||||||||||||||||||||||||||||||||
| corev1 "k8s.io/api/core/v1" | ||||||||||||||||||||||||||||||||||||||||||||||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||||||||||||||||||||||||||||||||||||||||||||||
| "k8s.io/apimachinery/pkg/types" | ||||||||||||||||||||||||||||||||||||||||||||||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||||||||||||||||||||||||||||||||||||||||||||||
| gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| kogateway "github.com/kong/kong-operator/controller/gateway" | ||||||||||||||||||||||||||||||||||||||||||||||
| certhelper "github.com/kong/kong-operator/ingress-controller/test/helpers/certificate" | ||||||||||||||||||||||||||||||||||||||||||||||
| gwcdecor "github.com/kong/kong-operator/internal/utils/gatewayclass" | ||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/kong/kong-operator/modules/manager/logging" | ||||||||||||||||||||||||||||||||||||||||||||||
| managerscheme "github.com/kong/kong-operator/modules/manager/scheme" | ||||||||||||||||||||||||||||||||||||||||||||||
| k8sutils "github.com/kong/kong-operator/pkg/utils/kubernetes" | ||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/kong/kong-operator/pkg/vars" | ||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| func TestGatewaySecretWatch_UpdatesResolvedRefsOnSecretRotation(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||||||||
| t.Parallel() | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| // Prepare scheme, envtest, manager and KO Gateway reconciler. | ||||||||||||||||||||||||||||||||||||||||||||||
| scheme := managerscheme.Get() | ||||||||||||||||||||||||||||||||||||||||||||||
| ctx := t.Context() | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| cfg, ns := Setup(t, ctx, scheme) | ||||||||||||||||||||||||||||||||||||||||||||||
| mgr, logs := NewManager(t, ctx, cfg, scheme) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| r := &kogateway.Reconciler{ | ||||||||||||||||||||||||||||||||||||||||||||||
| Client: mgr.GetClient(), | ||||||||||||||||||||||||||||||||||||||||||||||
| Scheme: scheme, | ||||||||||||||||||||||||||||||||||||||||||||||
| Namespace: ns.Name, | ||||||||||||||||||||||||||||||||||||||||||||||
| DefaultDataPlaneImage: "kong:latest", | ||||||||||||||||||||||||||||||||||||||||||||||
| LoggingMode: logging.DevelopmentMode, | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| StartReconcilers(ctx, t, mgr, logs, r) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| c := mgr.GetClient() | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| // Create a GatewayClass accepted by the controller. | ||||||||||||||||||||||||||||||||||||||||||||||
| gc := &gatewayv1.GatewayClass{ | ||||||||||||||||||||||||||||||||||||||||||||||
| ObjectMeta: metav1.ObjectMeta{Name: "gc-ko"}, | ||||||||||||||||||||||||||||||||||||||||||||||
| Spec: gatewayv1.GatewayClassSpec{ | ||||||||||||||||||||||||||||||||||||||||||||||
| ControllerName: gatewayv1.GatewayController(vars.ControllerName()), | ||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, c.Create(ctx, gc)) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| t.Log("patching GatewayClass status to Accepted=True") | ||||||||||||||||||||||||||||||||||||||||||||||
| require.Eventually(t, func() bool { | ||||||||||||||||||||||||||||||||||||||||||||||
| var cur gatewayv1.GatewayClass | ||||||||||||||||||||||||||||||||||||||||||||||
| if err := c.Get(ctx, types.NamespacedName{Name: gc.Name}, &cur); err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| cond := metav1.Condition{ | ||||||||||||||||||||||||||||||||||||||||||||||
| Type: string(gatewayv1.GatewayClassConditionStatusAccepted), | ||||||||||||||||||||||||||||||||||||||||||||||
| Status: metav1.ConditionTrue, | ||||||||||||||||||||||||||||||||||||||||||||||
| Reason: string(gatewayv1.GatewayClassReasonAccepted), | ||||||||||||||||||||||||||||||||||||||||||||||
| ObservedGeneration: cur.Generation, | ||||||||||||||||||||||||||||||||||||||||||||||
| LastTransitionTime: metav1.Now(), | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| // Use shared helper to set/merge the condition. | ||||||||||||||||||||||||||||||||||||||||||||||
| gwcd := gwcdecor.DecorateGatewayClass(&cur) | ||||||||||||||||||||||||||||||||||||||||||||||
| k8sutils.SetCondition(cond, gwcd) | ||||||||||||||||||||||||||||||||||||||||||||||
| if err := c.Status().Update(ctx, &cur); err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| return true | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+55
to
+73
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks like something that we could reuse. We already have a bunch of similar helpers in https://github.com/Kong/kong-operator/blob/e90fd984547cd60e3ce8c481f368514dd1936411/pkg/utils/test/predicates.go, e.g. kong-operator/pkg/utils/test/predicates.go Lines 463 to 484 in e90fd98
I think it would make sense to put it there and use it here. WDYT? |
||||||||||||||||||||||||||||||||||||||||||||||
| }, waitTime, tickTime) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| // Create an initial INVALID TLS Secret referenced by the Gateway listener. | ||||||||||||||||||||||||||||||||||||||||||||||
| secretName := "test-cert" | ||||||||||||||||||||||||||||||||||||||||||||||
| bad := &corev1.Secret{ | ||||||||||||||||||||||||||||||||||||||||||||||
| ObjectMeta: metav1.ObjectMeta{Namespace: ns.Name, Name: secretName}, | ||||||||||||||||||||||||||||||||||||||||||||||
| Type: corev1.SecretTypeTLS, | ||||||||||||||||||||||||||||||||||||||||||||||
| Data: map[string][]byte{ | ||||||||||||||||||||||||||||||||||||||||||||||
| corev1.TLSCertKey: []byte("not-a-cert"), | ||||||||||||||||||||||||||||||||||||||||||||||
| corev1.TLSPrivateKeyKey: []byte("not-a-key"), | ||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, c.Create(ctx, bad)) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| // Create a Gateway with a TLS listener referencing the Secret. | ||||||||||||||||||||||||||||||||||||||||||||||
| gw := &gatewayv1.Gateway{ | ||||||||||||||||||||||||||||||||||||||||||||||
| ObjectMeta: metav1.ObjectMeta{Namespace: ns.Name, Name: "gw"}, | ||||||||||||||||||||||||||||||||||||||||||||||
| Spec: gatewayv1.GatewaySpec{ | ||||||||||||||||||||||||||||||||||||||||||||||
| GatewayClassName: gatewayv1.ObjectName(gc.Name), | ||||||||||||||||||||||||||||||||||||||||||||||
| Listeners: []gatewayv1.Listener{ | ||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||
| Name: "https", | ||||||||||||||||||||||||||||||||||||||||||||||
| Port: 443, | ||||||||||||||||||||||||||||||||||||||||||||||
| Protocol: gatewayv1.HTTPSProtocolType, | ||||||||||||||||||||||||||||||||||||||||||||||
| TLS: &gatewayv1.ListenerTLSConfig{ | ||||||||||||||||||||||||||||||||||||||||||||||
| Mode: lo.ToPtr(gatewayv1.TLSModeTerminate), | ||||||||||||||||||||||||||||||||||||||||||||||
| CertificateRefs: []gatewayv1.SecretObjectReference{{ | ||||||||||||||||||||||||||||||||||||||||||||||
| Name: gatewayv1.ObjectName(secretName), | ||||||||||||||||||||||||||||||||||||||||||||||
| }}, | ||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, c.Create(ctx, gw)) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| t.Log("verifying that the invalid Secret results in ResolvedRefs=False (InvalidCertificateRef)") | ||||||||||||||||||||||||||||||||||||||||||||||
| waitForGatewayResolvedRefsStatus(t, ctx, c, ns.Name, gw.Name, metav1.ConditionFalse) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| t.Log("rotating the Secret with a valid TLS certificate and key") | ||||||||||||||||||||||||||||||||||||||||||||||
| certPEM, keyPEM := certhelper.MustGenerateCertPEMFormat() | ||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, c.Patch(ctx, &corev1.Secret{ | ||||||||||||||||||||||||||||||||||||||||||||||
| ObjectMeta: metav1.ObjectMeta{Namespace: ns.Name, Name: secretName}, | ||||||||||||||||||||||||||||||||||||||||||||||
| Type: corev1.SecretTypeTLS, | ||||||||||||||||||||||||||||||||||||||||||||||
| Data: map[string][]byte{ | ||||||||||||||||||||||||||||||||||||||||||||||
| corev1.TLSCertKey: certPEM, | ||||||||||||||||||||||||||||||||||||||||||||||
| corev1.TLSPrivateKeyKey: keyPEM, | ||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||
| }, client.MergeFrom(bad))) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| t.Log("verifying that after rotation the Secret watch enqueues the Gateway and ResolvedRefs becomes True") | ||||||||||||||||||||||||||||||||||||||||||||||
| waitForGatewayResolvedRefsStatus(t, ctx, c, ns.Name, gw.Name, metav1.ConditionTrue) | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| // waitForGatewayResolvedRefsStatus waits until the Gateway's listener ResolvedRefs condition | ||||||||||||||||||||||||||||||||||||||||||||||
| // matches the expected status, or fails after the default timeout. | ||||||||||||||||||||||||||||||||||||||||||||||
| func waitForGatewayResolvedRefsStatus( | ||||||||||||||||||||||||||||||||||||||||||||||
| t *testing.T, | ||||||||||||||||||||||||||||||||||||||||||||||
| ctx context.Context, | ||||||||||||||||||||||||||||||||||||||||||||||
| c client.Client, | ||||||||||||||||||||||||||||||||||||||||||||||
| namespace, name string, | ||||||||||||||||||||||||||||||||||||||||||||||
| status metav1.ConditionStatus, | ||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||
| t.Helper() | ||||||||||||||||||||||||||||||||||||||||||||||
| require.Eventually(t, func() bool { | ||||||||||||||||||||||||||||||||||||||||||||||
| var cur gatewayv1.Gateway | ||||||||||||||||||||||||||||||||||||||||||||||
| if err := c.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, &cur); err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| if len(cur.Status.Listeners) == 0 { | ||||||||||||||||||||||||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| for _, ls := range cur.Status.Listeners { | ||||||||||||||||||||||||||||||||||||||||||||||
| for _, cond := range ls.Conditions { | ||||||||||||||||||||||||||||||||||||||||||||||
| if cond.Type == string(gatewayv1.ListenerConditionResolvedRefs) && cond.Status == status { | ||||||||||||||||||||||||||||||||||||||||||||||
| return true | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||||||||||||||||||||||||
| }, waitTime, tickTime) | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+128
to
+155
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto about defining a reusable test assertion helper. |
||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYT about extracting this into a function and adding a test for it?