Skip to content

Commit

Permalink
Merge branch 'main' into dependabot/github_actions/docker/build-push-…
Browse files Browse the repository at this point in the history
…action-5.3.0
  • Loading branch information
pmalek authored Mar 15, 2024
2 parents 32bc45e + 00f85d5 commit 9061a4f
Show file tree
Hide file tree
Showing 7 changed files with 149 additions and 36 deletions.
1 change: 1 addition & 0 deletions CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* @Kong/k8s-maintainers
72 changes: 44 additions & 28 deletions controllers/controlplane/controller_reconciler_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,28 +319,34 @@ func (r *Reconciler) ensureClusterRole(
}

controlplaneContainer := k8sutils.GetPodContainerByName(&controlplane.Spec.Deployment.PodTemplateSpec.Spec, consts.ControlPlaneControllerContainerName)
generatedClusterRole, err := k8sresources.GenerateNewClusterRoleForControlPlane(controlplane.Name, controlplaneContainer.Image, r.DevelopmentMode)
generated, err := k8sresources.GenerateNewClusterRoleForControlPlane(controlplane.Name, controlplaneContainer.Image, r.DevelopmentMode)
if err != nil {
return false, nil, err
}
k8sutils.SetOwnerForObject(generatedClusterRole, controlplane)
k8sutils.SetOwnerForObject(generated, controlplane)

if count == 1 {
var updated bool
existingClusterRole := &clusterRoles[0]
updated, generatedClusterRole.ObjectMeta = k8sutils.EnsureObjectMetaIsUpdated(existingClusterRole.ObjectMeta, generatedClusterRole.ObjectMeta)
var (
updated bool
existing = &clusterRoles[0]
old = existing.DeepCopy()
)

updated, existing.ObjectMeta = k8sutils.EnsureObjectMetaIsUpdated(existing.ObjectMeta, generated.ObjectMeta)
if updated ||
!cmp.Equal(existingClusterRole.Rules, generatedClusterRole.Rules) ||
!cmp.Equal(existingClusterRole.AggregationRule, generatedClusterRole.AggregationRule) {
if err := r.Client.Patch(ctx, generatedClusterRole, client.MergeFrom(existingClusterRole)); err != nil {
return false, existingClusterRole, fmt.Errorf("failed patching ControlPlane's ClusterRole %s: %w", existingClusterRole.Name, err)
!cmp.Equal(existing.Rules, generated.Rules) ||
!cmp.Equal(existing.AggregationRule, generated.AggregationRule) {
existing.Rules = generated.Rules
existing.AggregationRule = generated.AggregationRule
if err := r.Client.Patch(ctx, existing, client.MergeFrom(old)); err != nil {
return false, existing, fmt.Errorf("failed patching ControlPlane's ClusterRole %s: %w", existing.Name, err)
}
return true, generatedClusterRole, nil
return true, existing, nil
}
return false, generatedClusterRole, nil
return false, existing, nil
}

return true, generatedClusterRole, r.Client.Create(ctx, generatedClusterRole)
return true, generated, r.Client.Create(ctx, generated)
}

func (r *Reconciler) ensureClusterRoleBinding(
Expand Down Expand Up @@ -371,37 +377,47 @@ func (r *Reconciler) ensureClusterRoleBinding(
return false, nil, errors.New("number of clusterRoleBindings reduced")
}

generatedClusterRoleBinding := k8sresources.GenerateNewClusterRoleBindingForControlPlane(controlplane.Namespace, controlplane.Name, serviceAccountName, clusterRoleName)
k8sutils.SetOwnerForObject(generatedClusterRoleBinding, controlplane)
generated := k8sresources.GenerateNewClusterRoleBindingForControlPlane(controlplane.Namespace, controlplane.Name, serviceAccountName, clusterRoleName)
k8sutils.SetOwnerForObject(generated, controlplane)

if count == 1 {
existingClusterRoleBinding := &clusterRoleBindings[0]
existing := &clusterRoleBindings[0]
// Delete and re-create ClusterRoleBinding if name of ClusterRole changed because RoleRef is immutable.
if !k8sresources.CompareClusterRoleName(existingClusterRoleBinding, clusterRoleName) {
if !k8sresources.CompareClusterRoleName(existing, clusterRoleName) {
log.Debug(logger, "ClusterRole name changed, delete and re-create a ClusterRoleBinding",
existingClusterRoleBinding,
"old_cluster_role", existingClusterRoleBinding.RoleRef.Name,
existing,
"old_cluster_role", existing.RoleRef.Name,
"new_cluster_role", clusterRoleName,
)
if err := r.Client.Delete(ctx, existingClusterRoleBinding); err != nil {
if err := r.Client.Delete(ctx, existing); err != nil {
return false, nil, err
}
return false, nil, errors.New("name of ClusterRole changed, out of date ClusterRoleBinding deleted")
}
var updated bool
updated, generatedClusterRoleBinding.ObjectMeta = k8sutils.EnsureObjectMetaIsUpdated(existingClusterRoleBinding.ObjectMeta, generatedClusterRoleBinding.ObjectMeta)
if updated ||
!k8sresources.ClusterRoleBindingContainsServiceAccount(existingClusterRoleBinding, controlplane.Namespace, serviceAccountName) {
if err := r.Client.Patch(ctx, generatedClusterRoleBinding, client.MergeFrom(existingClusterRoleBinding)); err != nil {
return false, existingClusterRoleBinding, fmt.Errorf("failed patching ControlPlane's ClusterRoleBinding %s: %w", existingClusterRoleBinding.Name, err)

var (
old = existing.DeepCopy()
updated bool
updatedServiceAccount bool
)
updated, existing.ObjectMeta = k8sutils.EnsureObjectMetaIsUpdated(existing.ObjectMeta, generated.ObjectMeta)

if !k8sresources.ClusterRoleBindingContainsServiceAccount(existing, controlplane.Namespace, serviceAccountName) {
existing.Subjects = generated.Subjects
updatedServiceAccount = true
}

if updated || updatedServiceAccount {
if err := r.Client.Patch(ctx, existing, client.MergeFrom(old)); err != nil {
return false, existing, fmt.Errorf("failed patching ControlPlane's ClusterRoleBinding %s: %w", existing.Name, err)
}
return true, generatedClusterRoleBinding, nil
return true, existing, nil
}
return false, generatedClusterRoleBinding, nil
return false, existing, nil

}

return true, generatedClusterRoleBinding, r.Client.Create(ctx, generatedClusterRoleBinding)
return true, generated, r.Client.Create(ctx, generated)
}

// ensureAdminMTLSCertificateSecret ensures that a Secret is created with the certificate for mTLS communication between the
Expand Down
43 changes: 43 additions & 0 deletions pkg/utils/test/predicates.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"io"
"net/http"
"slices"
"strings"
"testing"

Expand All @@ -13,6 +14,7 @@ import (
autoscalingv2 "k8s.io/api/autoscaling/v2"
corev1 "k8s.io/api/core/v1"
netv1 "k8s.io/api/networking/v1"
rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -191,6 +193,47 @@ func ControlPlaneHasClusterRole(t *testing.T, ctx context.Context, controlplane
}
}

// ControlPlanesClusterRoleHasPolicyRule is a helper function for tests that returns a function
// that can be used to check if ControlPlane's ClusterRole contains the provided PolicyRule.
// Should be used in conjunction with require.Eventually or assert.Eventually.
func ControlPlanesClusterRoleHasPolicyRule(t *testing.T, ctx context.Context, controlplane *operatorv1beta1.ControlPlane, clients K8sClients, pr rbacv1.PolicyRule) func() bool {
return func() bool {
clusterRoles := MustListControlPlaneClusterRoles(t, ctx, controlplane, clients)
t.Logf("%d clusterroles", len(clusterRoles))
if len(clusterRoles) == 0 {
return false
}
t.Logf("got %s clusterrole, checking if it contains the requested PolicyRule", clusterRoles[0].Name)
return slices.ContainsFunc(clusterRoles[0].Rules, func(e rbacv1.PolicyRule) bool {
return slices.Equal(e.APIGroups, pr.APIGroups) &&
slices.Equal(e.ResourceNames, pr.ResourceNames) &&
slices.Equal(e.Resources, pr.Resources) &&
slices.Equal(e.Verbs, pr.Verbs) &&
slices.Equal(e.NonResourceURLs, pr.NonResourceURLs)
})
}
}

// ControlPlanesClusterRoleBindingHasSubject is a helper function for tests that returns a function
// that can be used to check if ControlPlane's ClusterRoleBinding contains the provided Subject.
// Should be used in conjunction with require.Eventually or assert.Eventually.
func ControlPlanesClusterRoleBindingHasSubject(t *testing.T, ctx context.Context, controlplane *operatorv1beta1.ControlPlane, clients K8sClients, subject rbacv1.Subject) func() bool {
return func() bool {
clusterRoleBindings := MustListControlPlaneClusterRoleBindings(t, ctx, controlplane, clients)
t.Logf("%d clusterrolesbindings", len(clusterRoleBindings))
if len(clusterRoleBindings) == 0 {
return false
}
t.Logf("got %s clusterrolebinding, checking if it contains the requested Subject", clusterRoleBindings[0].Name)
return slices.ContainsFunc(clusterRoleBindings[0].Subjects, func(e rbacv1.Subject) bool {
return e.Kind == subject.Kind &&
e.APIGroup == subject.APIGroup &&
e.Name == subject.Name &&
e.Namespace == subject.Namespace
})
}
}

// ControlPlaneHasClusterRoleBinding is a helper function for tests that returns a function
// that can be used to check if a ControlPlane has a ClusterRoleBinding.
// Should be used in conjunction with require.Eventually or assert.Eventually.
Expand Down
49 changes: 47 additions & 2 deletions test/integration/test_controlplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"reflect"
"slices"
"testing"
"time"

Expand All @@ -12,6 +13,7 @@ import (
"github.com/stretchr/testify/require"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -169,6 +171,13 @@ func TestControlPlaneEssentials(t *testing.T) {
{
Name: consts.DataPlaneProxyContainerName,
Image: helpers.GetDefaultDataPlaneImage(),
// Speed up the test.
ReadinessProbe: func() *corev1.Probe {
p := k8sresources.GenerateDataPlaneReadinessProbe(consts.DataPlaneStatusEndpoint)
p.InitialDelaySeconds = 1
p.PeriodSeconds = 1
return p
}(),
},
},
},
Expand Down Expand Up @@ -209,6 +218,13 @@ func TestControlPlaneEssentials(t *testing.T) {
},
Name: consts.ControlPlaneControllerContainerName,
Image: consts.DefaultControlPlaneImage,
// Speed up the test.
ReadinessProbe: func() *corev1.Probe {
p := k8sresources.GenerateControlPlaneProbe("/readyz", intstr.FromInt(10254))
p.InitialDelaySeconds = 1
p.PeriodSeconds = 1
return p
}(),
},
},
},
Expand Down Expand Up @@ -325,10 +341,39 @@ func TestControlPlaneEssentials(t *testing.T) {
t.Log("verifying controlplane's webhook is functional")
verifyControlPlaneWebhookIsFunctional(t, GetCtx(), clients)

t.Log("verifying that controlplane's ClusterRole is patched if it goes out of sync")
clusterRoles = testutils.MustListControlPlaneClusterRoles(t, GetCtx(), controlplane, clients)
require.Len(t, clusterRoles, 1, "There must be only one ControlPlane ClusterRole")
clusterRole := clusterRoles[0]
idx := slices.IndexFunc(clusterRole.Rules, func(pr rbacv1.PolicyRule) bool {
return pr.Resources != nil && slices.Contains(pr.Resources, "endpointslices")
})
require.GreaterOrEqual(t, idx, 0)
endpointSlicesRule := clusterRole.Rules[idx]
oldClusterRole := clusterRole.DeepCopy()
require.NotEmpty(t, clusterRole.Rules)
clusterRole.Rules = slices.Delete(clusterRole.Rules, idx, idx+1)
t.Logf("deleting endpointslices policyrule form %s clusterrole", clusterRole.Name)
require.NoError(t, clients.MgrClient.Patch(GetCtx(), &clusterRole, client.MergeFrom(oldClusterRole)))
t.Log("verifying that controlplane's ClusterRole is patched with the policy rule that was removed")
require.Eventually(t, testutils.ControlPlanesClusterRoleHasPolicyRule(t, GetCtx(), controlplane, clients, endpointSlicesRule), testutils.ControlPlaneCondDeadline, testutils.ControlPlaneCondTick)

t.Log("verifying that controlplane's ClusterRoleBinding is patched if it goes out of sync")
clusterRoleBindings = testutils.MustListControlPlaneClusterRoleBindings(t, GetCtx(), controlplane, clients)
require.Len(t, clusterRoleBindings, 1, "There must be only one ControlPlane ClusterRoleBinding")
clusterRoleBinding := clusterRoleBindings[0]
require.NotEmpty(t, clusterRoleBinding.Subjects)
subject := clusterRoleBinding.Subjects[0]
oldClusterRoleBinding := clusterRoleBinding.DeepCopy()
clusterRoleBinding.Subjects = slices.Delete(clusterRoleBinding.Subjects, 0, 1)
t.Logf("deleting %s/%s subject form %s clusterrolebinding", subject.Namespace, subject.Name, clusterRoleBinding.Name)
require.NoError(t, clients.MgrClient.Patch(GetCtx(), &clusterRoleBinding, client.MergeFrom(oldClusterRoleBinding)))
t.Log("verifying that controlplane's ClusterRoleBinding is patched with the subject that was removed")
require.Eventually(t, testutils.ControlPlanesClusterRoleBindingHasSubject(t, GetCtx(), controlplane, clients, subject), testutils.ControlPlaneCondDeadline, testutils.ControlPlaneCondTick)

// delete controlplane and verify that cluster wide resources removed.
t.Log("verifying cluster wide resources removed after controlplane deleted")
err = controlplaneClient.Delete(GetCtx(), controlplane.Name, metav1.DeleteOptions{})
require.NoError(t, err)
require.NoError(t, controlplaneClient.Delete(GetCtx(), controlplane.Name, metav1.DeleteOptions{}))
require.Eventually(t, testutils.Not(testutils.ControlPlaneHasClusterRole(t, GetCtx(), controlplane, clients)), testutils.ControlPlaneCondDeadline, testutils.ControlPlaneCondTick)
require.Eventually(t, testutils.Not(testutils.ControlPlaneHasClusterRoleBinding(t, GetCtx(), controlplane, clients)), testutils.ControlPlaneCondDeadline, testutils.ControlPlaneCondTick)
require.Eventually(t, testutils.Not(testutils.ControlPlaneHasAdmissionWebhookConfiguration(t, GetCtx(), controlplane, clients)), testutils.ControlPlaneCondDeadline, testutils.ControlPlaneCondTick)
Expand Down
14 changes: 11 additions & 3 deletions test/integration/test_httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/google/uuid"
"github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/generators"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -156,9 +157,16 @@ func TestHTTPRouteV1Beta1(t *testing.T) {
},
}
t.Logf("creating httproute %s/%s to access deployment %s via kong", httpRoute.Namespace, httpRoute.Name, deployment.Name)
httpRoute, err = GetClients().GatewayClient.GatewayV1().HTTPRoutes(namespace.Name).
Create(GetCtx(), httpRoute, metav1.CreateOptions{})
require.NoError(t, err)
require.EventuallyWithT(t,
func(c *assert.CollectT) {
httpRoute, err = GetClients().GatewayClient.GatewayV1().HTTPRoutes(namespace.Name).Create(GetCtx(), httpRoute, metav1.CreateOptions{})
if err != nil {
t.Logf("failed to deploy httproute: %v", err)
c.Errorf("failed to deploy httproute: %v", err)
}
},
testutils.DefaultIngressWait, testutils.WaitIngressTick,
)
cleaner.Add(httpRoute)

t.Log("verifying connectivity to the HTTPRoute")
Expand Down
2 changes: 1 addition & 1 deletion third_party/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ go 1.22
toolchain go1.22.0

require (
github.com/elastic/crd-ref-docs v0.0.10
github.com/elastic/crd-ref-docs v0.0.11
github.com/go-delve/delve v1.22.1
github.com/golangci/golangci-lint v1.56.2
github.com/operator-framework/operator-registry v1.36.0
Expand Down
4 changes: 2 additions & 2 deletions third_party/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,8 @@ github.com/docker/go-units v0.5.0 h1:69rxXcBk27SvSaaxTtLh/8llcHD8vYHT7WSdRZ/jvr4
github.com/docker/go-units v0.5.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk=
github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7 h1:UhxFibDNY/bfvqU5CAUmr9zpesgbU6SWc8/B4mflAE4=
github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7/go.mod h1:cyGadeNEkKy96OOhEzfZl+yxihPEzKnqJwvfuSUqbZE=
github.com/elastic/crd-ref-docs v0.0.10 h1:FAc9oCxxY4+rMCLSLtTGrEaPyuxmp3LNlQ+dZfG9Ujc=
github.com/elastic/crd-ref-docs v0.0.10/go.mod h1:zha4djxzWirfx+c4fl/Kmk9Rc7Fv7XBoOi9CL9kne+M=
github.com/elastic/crd-ref-docs v0.0.11 h1:/wNYycaIOQSnWPDdteRnx3IqvTBTzieQ6PHJS+g6nO4=
github.com/elastic/crd-ref-docs v0.0.11/go.mod h1:B4kHxcBLQ41TbGnUTjdliNmuG6Dv7MFrH2pmYTjN4IQ=
github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxERmMY4rD+g=
github.com/emicklei/go-restful/v3 v3.11.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc=
github.com/emirpasic/gods v1.12.0/go.mod h1:YfzfFFoVP/catgzJb4IKIqXjX78Ha8FMSDh3ymbK86o=
Expand Down

0 comments on commit 9061a4f

Please sign in to comment.