Skip to content

Commit

Permalink
Get ServiceAccount tokens with TokenRequest API (#951)
Browse files Browse the repository at this point in the history
* Get ServiceAccount tokens with TokenRequest API

This is a necessary change to support Kubernetes 1.24+

See: https://itnext.io/big-change-in-k8s-1-24-about-serviceaccounts-and-their-secrets-4b909a4af4e0

* Force upgrade test to use prior KIND version.

Even though we want to test against 1.24.x the prior release doesn't work on this. We can undo this, once we have an official release that supports 1.24

Fixes [#923]
  • Loading branch information
idoru authored Aug 4, 2022
1 parent 91e5754 commit 71dde00
Show file tree
Hide file tree
Showing 32 changed files with 6,278 additions and 456 deletions.
9 changes: 5 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ ADDLICENSE ?= go run -modfile hack/tools/go.mod github.com/google/addlicense
GOLANGCI_LINT ?= go run -modfile hack/tools/go.mod github.com/golangci/golangci-lint/cmd/golangci-lint
GINKGO ?= go run -modfile hack/tools/go.mod github.com/onsi/ginkgo/ginkgo
GCI_LINT ?= go run -modfile hack/tools/go.mod github.com/daixiang0/gci
CONTROL_PLANE_KUTTL ?= kubectl kuttl test --control-plane-config=kuttl-control-plane.config

ifndef ($LOG_LEVEL)
# set a default LOG_LEVEL whenever we run the controller
Expand Down Expand Up @@ -94,19 +95,19 @@ test-integration: test-gen-manifests test-gen-objects

.PHONY: test-kuttl
test-kuttl: build test-gen-manifests
if [ -n "$$focus" ]; then kubectl kuttl test --test $$(basename $(focus)); else kubectl kuttl test; fi
if [ -n "$$focus" ]; then $(CONTROL_PLANE_KUTTL) --test $$(basename $(focus)); else $(CONTROL_PLANE_KUTTL); fi

.PHONY: test-kuttl-runnable
test-kuttl-runnable: build test-gen-manifests
if [ -n "$$focus" ]; then kubectl kuttl test ./tests/kuttl/runnable --test $$(basename $(focus)); else kubectl kuttl test ./tests/kuttl/runnable; fi
if [ -n "$$focus" ]; then $(CONTROL_PLANE_KUTTL) ./tests/kuttl/runnable --test $$(basename $(focus)); else $(CONTROL_PLANE_KUTTL) ./tests/kuttl/runnable; fi

.PHONY: test-kuttl-supplychain
test-kuttl-supplychain: build test-gen-manifests
if [ -n "$$focus" ]; then kubectl kuttl test ./tests/kuttl/supplychain --test $$(basename $(focus)); else kubectl kuttl test ./tests/kuttl/supplychain; fi
if [ -n "$$focus" ]; then $(CONTROL_PLANE_KUTTL) ./tests/kuttl/supplychain --test $$(basename $(focus)); else $(CONTROL_PLANE_KUTTL) ./tests/kuttl/supplychain; fi

.PHONY: test-kuttl-delivery
test-kuttl-delivery: build test-gen-manifests
if [ -n "$$focus" ]; then kubectl kuttl test ./tests/kuttl/delivery --test $$(basename $(focus)); else kubectl kuttl test ./tests/kuttl/delivery; fi
if [ -n "$$focus" ]; then $(CONTROL_PLANE_KUTTL) ./tests/kuttl/delivery --test $$(basename $(focus)); else $(CONTROL_PLANE_KUTTL) ./tests/kuttl/delivery; fi

.PHONY: list-kuttl
list-kuttl:
Expand Down
6 changes: 6 additions & 0 deletions config/rbac/rbac.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ kind: ClusterRole
metadata:
name: cartographer-controller-admin
rules:
- apiGroups:
- ""
resources:
- serviceaccounts/token
verbs:
- create
- apiGroups:
- carto.run
resources:
Expand Down
2 changes: 1 addition & 1 deletion hack/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ readonly DIR="$(cd "$(dirname "$0")" && pwd)"
readonly HOST_ADDR=${HOST_ADDR:-$("$DIR"/ip.py)}
readonly REGISTRY_PORT=${REGISTRY_PORT:-5001}
readonly REGISTRY=${REGISTRY:-"${HOST_ADDR}:${REGISTRY_PORT}"}
readonly KIND_IMAGE=${KIND_IMAGE:-kindest/node:v1.21.1}
readonly KIND_IMAGE=${KIND_IMAGE:-kindest/node:v1.24.2}
readonly RELEASE_VERSION=${RELEASE_VERSION:-""}
readonly RELEASE_YAML_PATH=${RELEASE_YAML_PATH:-"./release/cartographer.yaml"}
# shellcheck disable=SC2034 # This _should_ be marked as an extern but I clearly don't understand how it operates in github actions
Expand Down
2 changes: 1 addition & 1 deletion hack/upgrade-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ readonly CONFIG_BRANCH="main"
readonly CONFIG_COMMIT_MESSAGE="Update config"

main() {
"$DIR/setup.sh" cluster example-dependencies
KIND_IMAGE='kindest/node:v1.21.1' "$DIR/setup.sh" cluster example-dependencies
install_latest_released_cartographer

port=$(available_port)
Expand Down
12 changes: 12 additions & 0 deletions kuttl-control-plane.config
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--advertise-address=127.0.0.1
--etcd-servers={{ if .EtcdURL }}{{ .EtcdURL.String }}{{ end }}
--cert-dir={{ .CertDir }}
--insecure-port={{ if .URL }}{{ .URL.Port }}{{ end }}
--insecure-bind-address={{ if .URL }}{{ .URL.Hostname }}{{ end }}
--secure-port={{ if .SecurePort }}{{ .SecurePort }}{{ end }}
--disable-admission-plugins=ServiceAccount,NamespaceLifecycle
--service-cluster-ip-range=10.0.0.0/24
--advertise-address={{ if .URL }}{{ .URL.Hostname }}{{ end }}
--service-account-issuer=https://kubernetes.default.svc.cluster.local
--service-account-key-file={{ .CertDir }}/apiserver.crt
--service-account-signing-key-file={{ .CertDir }}/apiserver.key
3 changes: 2 additions & 1 deletion pkg/apis/v1alpha1/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ const (
// -- OWNER ConditionType - ResourcesSubmitted ConditionReasons

const (
ServiceAccountSecretErrorResourcesSubmittedReason = "ServiceAccountSecretError"
ServiceAccountErrorResourcesSubmittedReason = "ServiceAccountError"
ServiceAccountTokenErrorResourcesSubmittedReason = "ServiceAccountTokenError"
ResourceRealizerBuilderErrorResourcesSubmittedReason = "ResourceRealizerBuilderError"
)

Expand Down
13 changes: 11 additions & 2 deletions pkg/conditions/owner_conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,20 @@ func getConditionType(isOwner bool) string {

// -- Owner.Status.Conditions - ResourcesSubmitted

func ServiceAccountSecretNotFoundCondition(err error) metav1.Condition {
func ServiceAccountNotFoundCondition(err error) metav1.Condition {
return metav1.Condition{
Type: v1alpha1.OwnerResourcesSubmitted,
Status: metav1.ConditionFalse,
Reason: v1alpha1.ServiceAccountSecretErrorResourcesSubmittedReason,
Reason: v1alpha1.ServiceAccountErrorResourcesSubmittedReason,
Message: err.Error(),
}
}

func ServiceAccountTokenErrorCondition(err error) metav1.Condition {
return metav1.Condition{
Type: v1alpha1.OwnerResourcesSubmitted,
Status: metav1.ConditionFalse,
Reason: v1alpha1.ServiceAccountTokenErrorResourcesSubmittedReason,
Message: err.Error(),
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/conditions/runnable_conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func RunnableServiceAccountSecretNotFoundCondition(err error) metav1.Condition {
return metav1.Condition{
Type: v1alpha1.RunTemplateReady,
Status: metav1.ConditionFalse,
Reason: v1alpha1.ServiceAccountSecretErrorResourcesSubmittedReason,
Reason: v1alpha1.ServiceAccountErrorResourcesSubmittedReason,
Message: err.Error(),
}
}
Expand Down
23 changes: 19 additions & 4 deletions pkg/controllers/deliverable_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes"
"sigs.k8s.io/cluster-api/controllers/external"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -42,13 +43,15 @@ import (
"github.com/vmware-tanzu/cartographer/pkg/realizer/healthcheck"
"github.com/vmware-tanzu/cartographer/pkg/realizer/statuses"
"github.com/vmware-tanzu/cartographer/pkg/repository"
"github.com/vmware-tanzu/cartographer/pkg/satoken"
"github.com/vmware-tanzu/cartographer/pkg/templates"
"github.com/vmware-tanzu/cartographer/pkg/tracker/dependency"
"github.com/vmware-tanzu/cartographer/pkg/tracker/stamped"
"github.com/vmware-tanzu/cartographer/pkg/utils"
)

type DeliverableReconciler struct {
TokenManager satoken.TokenManager
Repo repository.Repository
ConditionManagerBuilder conditions.ConditionManagerBuilder
ResourceRealizerBuilder realizer.ResourceRealizerBuilder
Expand Down Expand Up @@ -111,13 +114,19 @@ func (r *DeliverableReconciler) Reconcile(ctx context.Context, req ctrl.Request)

serviceAccountName, serviceAccountNS := getServiceAccountNameAndNamespaceForDeliverable(deliverable, delivery)

secret, err := r.Repo.GetServiceAccountSecret(ctx, serviceAccountName, serviceAccountNS)
serviceAccount, err := r.Repo.GetServiceAccount(ctx, serviceAccountName, serviceAccountNS)
if err != nil {
r.conditionManager.AddPositive(conditions.ServiceAccountSecretNotFoundCondition(err))
return r.completeReconciliation(ctx, deliverable, nil, fmt.Errorf("failed to get secret for service account [%s]: %w", fmt.Sprintf("%s/%s", serviceAccountNS, serviceAccountName), err))
r.conditionManager.AddPositive(conditions.ServiceAccountNotFoundCondition(err))
return r.completeReconciliation(ctx, deliverable, nil, fmt.Errorf("failed to get service account [%s]: %w", fmt.Sprintf("%s/%s", req.Namespace, serviceAccountName), err))
}

resourceRealizer, err := r.ResourceRealizerBuilder(secret, deliverable, deliverable.Spec.Params, r.Repo, delivery.Spec.Params, buildDeliverableResourceLabeler(deliverable, delivery))
saToken, err := r.TokenManager.GetServiceAccountToken(serviceAccount)
if err != nil {
r.conditionManager.AddPositive(conditions.ServiceAccountTokenErrorCondition(err))
return r.completeReconciliation(ctx, deliverable, nil, fmt.Errorf("failed to get token for service account [%s]: %w", fmt.Sprintf("%s/%s", serviceAccountNS, serviceAccountName), err))
}

resourceRealizer, err := r.ResourceRealizerBuilder(saToken, deliverable, deliverable.Spec.Params, r.Repo, delivery.Spec.Params, buildDeliverableResourceLabeler(deliverable, delivery))

if err != nil {
r.conditionManager.AddPositive(conditions.ResourceRealizerBuilderErrorCondition(err))
Expand Down Expand Up @@ -379,6 +388,12 @@ func getServiceAccountNameAndNamespaceForDeliverable(deliverable *v1alpha1.Deliv
}

func (r *DeliverableReconciler) SetupWithManager(mgr ctrl.Manager) error {
clientSet, err := kubernetes.NewForConfig(mgr.GetConfig())
if err != nil {
return err
}
r.TokenManager = satoken.NewManager(clientSet, mgr.GetLogger().WithName("service-account-token-manager"), nil)

r.Repo = repository.NewRepository(
mgr.GetClient(),
repository.NewCache(mgr.GetLogger().WithName("deliverable-repo-cache")),
Expand Down
Loading

0 comments on commit 71dde00

Please sign in to comment.