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

🐛 Only delele serviceaccount managed by the addon agent #147

Merged
Show file tree
Hide file tree
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
8 changes: 6 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ else
GOBIN=$(shell go env GOBIN)
endif

export CGO_ENABLED = 1
export DOCKER_BUILDER ?= docker
export CGO_ENABLED = 1
export GOFLAGS ?=

# Setting SHELL to bash allows bash commands to be executed by recipes.
Expand Down Expand Up @@ -113,7 +114,10 @@ rm -rf $$TMP_DIR ;\
endef

images:
docker build -t ${IMG_REGISTRY}/managed-serviceaccount:${IMAGE_TAG} -f Dockerfile .
$(DOCKER_BUILDER) build -t ${IMG_REGISTRY}/managed-serviceaccount:${IMAGE_TAG} -f Dockerfile .

images-amd64:
$(DOCKER_BUILDER) build --platform linux/amd64 -t ${IMG_REGISTRY}/managed-serviceaccount:${IMAGE_TAG} -f Dockerfile .

test-integration:
@echo "TODO: Run integration test"
Expand Down
24 changes: 21 additions & 3 deletions pkg/addon/agent/controller/token.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,33 @@
return reconcile.Result{}, errors.Wrapf(err, "fail to get managed serviceaccount")
}

sai := r.SpokeNativeClient.CoreV1().ServiceAccounts(r.SpokeNamespace)
if err := sai.Delete(ctx, request.Name, metav1.DeleteOptions{}); err != nil {
saclient := r.SpokeNativeClient.CoreV1().ServiceAccounts(r.SpokeNamespace)
sa, err := saclient.Get(ctx, request.Name, metav1.GetOptions{})
if err != nil {
if !apierrors.IsNotFound(err) {
// fail to get related serviceaccount, requeue
return reconcile.Result{}, errors.Wrapf(err, "fail to get related serviceaccount")
}

Check warning on line 84 in pkg/addon/agent/controller/token.go

View check run for this annotation

Codecov / codecov/patch

pkg/addon/agent/controller/token.go#L82-L84

Added lines #L82 - L84 were not covered by tests

logger.Info("Both ManagedServiceAccount and related ServiceAccount does not exist")
return reconcile.Result{}, nil
}

// check if the serviceacount is managed by the agent, if not, return
if sa.Labels[common.LabelKeyIsManagedServiceAccount] != "true" {

logger.Info("Related ServiceAccount is not managed by the agent, skip deletion")
return reconcile.Result{}, nil
}

if err := saclient.Delete(ctx, request.Name, metav1.DeleteOptions{}); err != nil {
if !apierrors.IsNotFound(err) {
// fail to delete related serviceaccount, requeue
return reconcile.Result{}, errors.Wrapf(err, "fail to delete related serviceaccount")
}
}

logger.Info("Both ManagedServiceAccount and related ServiceAccount does not exist")
logger.Info("Delete related ServiceAccount successfully")
return reconcile.Result{}, nil
}

Expand Down
40 changes: 38 additions & 2 deletions pkg/addon/agent/controller/token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@ import (
"k8s.io/client-go/rest"
clienttesting "k8s.io/client-go/testing"
"k8s.io/klog/v2"
authv1beta1 "open-cluster-management.io/managed-serviceaccount/apis/authentication/v1beta1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

authv1beta1 "open-cluster-management.io/managed-serviceaccount/apis/authentication/v1beta1"
"open-cluster-management.io/managed-serviceaccount/pkg/common"
)

func TestReconcile(t *testing.T) {
Expand All @@ -51,9 +53,33 @@ func TestReconcile(t *testing.T) {
validateFunc func(t *testing.T, hubClient client.Client, actions []clienttesting.Action)
}{
{
name: "not found",
name: "msa and sa both are not found",
validateFunc: func(t *testing.T, hubClient client.Client, actions []clienttesting.Action) {
assertActions(t, actions,
"get", // get service account
)
},
},
{
name: "msa not found, sa is not created by msa agent, skip delete",
spokeNamespace: clusterName,
sa: newServiceAccount(clusterName, msaName),
validateFunc: func(t *testing.T, hubClient client.Client, actions []clienttesting.Action) {
assertActions(t, actions,
"get", // get service account
)
},
},
{
name: "msa not found, sa is created by msa agent, delete",
spokeNamespace: clusterName,
sa: newServiceAccountWithLabels(clusterName, msaName,
map[string]string{
common.LabelKeyIsManagedServiceAccount: "true",
}),
validateFunc: func(t *testing.T, hubClient client.Client, actions []clienttesting.Action) {
assertActions(t, actions,
"get", // get service account
"delete", // delete service account
)
},
Expand Down Expand Up @@ -442,6 +468,16 @@ func newServiceAccount(namespace, name string) *corev1.ServiceAccount {
}
}

func newServiceAccountWithLabels(namespace, name string, labels map[string]string) *corev1.ServiceAccount {
return &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: namespace,
Labels: labels,
},
}
}

func newServiceAccountWithSecret(namespace, name string, secretName string) *corev1.ServiceAccount {
sa := newServiceAccount(namespace, name)
sa.Secrets = []corev1.ObjectReference{
Expand Down
Loading