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

fix: Support Kubernetes v1.24. Fixes #8320 #9680

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from 15 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
13 changes: 9 additions & 4 deletions .github/workflows/ci-build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ jobs:
${{ runner.os }}-go-
- name: Install and start K3S
run: |
curl -sfL https://get.k3s.io | INSTALL_K3S_VERSION=v1.21.2+k3s1 INSTALL_K3S_CHANNEL=stable INSTALL_K3S_EXEC=--docker K3S_KUBECONFIG_MODE=644 sh -
curl -sfL https://get.k3s.io | INSTALL_K3S_CHANNEL=stable INSTALL_K3S_EXEC=--docker K3S_KUBECONFIG_MODE=644 sh -
until kubectl --kubeconfig=/etc/rancher/k3s/k3s.yaml cluster-info ; do sleep 10s ; done
cp /etc/rancher/k3s/k3s.yaml /home/runner/.kubeconfig
echo "- name: fake_token_user" >> $KUBECONFIG
Expand All @@ -155,14 +155,19 @@ jobs:
- run: make cli STATIC_FILES=false
if: ${{matrix.test == 'test-api' || matrix.test == 'test-cli' || matrix.test == 'test-java-sdk' || matrix.test == 'test-python-sdk'}}
- run: make start PROFILE=${{matrix.profile}} AUTH_MODE=client STATIC_FILES=false LOG_LEVEL=info API=${{matrix.test == 'test-api' || matrix.test == 'test-cli' || matrix.test == 'test-java-sdk' || matrix.test == 'test-python-sdk'}} UI=false AZURE=true > /tmp/argo.log 2>&1 &
- run: make wait
timeout-minutes: 4
- name: make wait
# https://github.com/marketplace/actions/retry-step
uses: nick-fields/[email protected]
with:
timeout_minutes: 4
max_attempts: 4
command: make wait
- name: make ${{matrix.test}}
# https://github.com/marketplace/actions/retry-step
uses: nick-fields/[email protected]
with:
timeout_minutes: 20
max_attempts: 2
max_attempts: 4
command: make ${{matrix.test}} E2E_SUITE_TIMEOUT=20m STATIC_FILES=false AZURE=true
- if: ${{ failure() }}
run: |
Expand Down
1 change: 1 addition & 0 deletions .spelling
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ v1.0
v1.1
v1.2
v1.3
v1.24
v2
v2.10
v2.11
Expand Down
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,7 @@ mysql-cli:
test-cli: ./dist/argo

test-%:
kubectl apply -f examples/secrets
go test -failfast -v -timeout $(E2E_SUITE_TIMEOUT) -count 1 --tags $* -parallel $(E2E_PARALLEL) ./test/e2e

.PHONY: test-examples
Expand Down
12 changes: 10 additions & 2 deletions docs/access-token.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,16 @@ kubectl create rolebinding jenkins --role=jenkins --serviceaccount=argo:jenkins
You now need to get a token:

```bash
SECRET=$(kubectl get sa jenkins -o=jsonpath='{.secrets[0].name}')
ARGO_TOKEN="Bearer $(kubectl get secret $SECRET -o=jsonpath='{.data.token}' | base64 --decode)"
kubectl apply -f - <<EOF
apiVersion: v1
kind: Secret
metadata:
name: jenkins.service-account-token
annotations:
kubernetes.io/service-account.name: jenkins
type: kubernetes.io/service-account-token
EOF
ARGO_TOKEN="Bearer $(kubectl get secret jenkins.service-account-token -o=jsonpath='{.data.token}' | base64 --decode)"
echo $ARGO_TOKEN
Bearer ZXlKaGJHY2lPaUpTVXpJMU5pSXNJbXRwWkNJNkltS...
```
Expand Down
5 changes: 5 additions & 0 deletions docs/manually-create-secrets.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Kubernetes Secrets

As of Kubernetes v1.24, secrets are not automatically created for service accounts by default. [Find out how to create these yourself manually](https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/#manually-create-a-service-account-api-token).

Argo discovers your token by name, not annotation. They must be named `${serviceAccountName}.service-account-token`.
Comment on lines +3 to +5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could do better job explaining when users need to do this. When using Agent http template? Auth delegation in api server? Does this need to be mentioned in the SSO docs?

7 changes: 7 additions & 0 deletions examples/secrets/default.service-account-token-secret.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
apiVersion: v1
kind: Secret
metadata:
name: default.service-account-token
annotations:
kubernetes.io/service-account.name: default
type: kubernetes.io/service-account-token
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
apiVersion: v1
kind: Secret
metadata:
name: template-executor-executor-plugin.service-account-token
annotations:
kubernetes.io/service-account.name: template-executor-executor-plugin
type: kubernetes.io/service-account-token
12 changes: 10 additions & 2 deletions hack/access-token.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,18 @@ case $1 in
kubectl create sa jenkins
kubectl delete rolebinding jenkins --ignore-not-found
kubectl create rolebinding jenkins --role=jenkins --serviceaccount=argo:jenkins
kubectl apply -f - <<EOF
apiVersion: v1
kind: Secret
metadata:
name: jenkins.service-account-token
annotations:
kubernetes.io/service-account.name: jenkins
type: kubernetes.io/service-account-token
EOF
;;
get)
SECRET=$(kubectl get sa jenkins -o=jsonpath='{.secrets[0].name}')
ARGO_TOKEN="Bearer $(kubectl get secret $SECRET -o=jsonpath='{.data.token}' | base64 --decode)"
ARGO_TOKEN="Bearer $(kubectl get secret jenkins.service-account-token -o=jsonpath='{.data.token}' | base64 --decode)"

curl -s http://localhost:2746/api/v1/workflows/argo -H "Authorization: $ARGO_TOKEN" > /dev/null

Expand Down
1 change: 1 addition & 0 deletions hack/test-examples.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ set -eu -o pipefail

# Load the configmaps that contains the parameter values used for certain examples.
kubectl apply -f examples/configmaps/simple-parameters-configmap.yaml
kubectl apply -f examples/secrets/default.service-account-token-secret.yaml

echo "Checking for banned images..."
grep -lR 'workflows.argoproj.io/test' examples/* | while read f ; do
Expand Down
1 change: 1 addition & 0 deletions mkdocs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ nav:
- workflow-executors.md
- workflow-restrictions.md
- sidecar-injection.md
- manually-create-secrets.md
- Argo Server:
- argo-server.md
- argo-server-auth-mode.md
Expand Down
8 changes: 4 additions & 4 deletions server/auth/gatekeeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"sort"
"strconv"

"github.com/argoproj/argo-workflows/v3/util/secrets"

eventsource "github.com/argoproj/argo-events/pkg/client/eventsource/clientset/versioned"
sensor "github.com/argoproj/argo-events/pkg/client/sensor/clientset/versioned"
log "github.com/sirupsen/logrus"
Expand Down Expand Up @@ -316,10 +318,8 @@ func (s *gatekeeper) rbacAuthorization(ctx context.Context, claims *types.Claims
}

func (s *gatekeeper) authorizationForServiceAccount(ctx context.Context, serviceAccount *corev1.ServiceAccount) (string, error) {
if len(serviceAccount.Secrets) == 0 {
return "", fmt.Errorf("expected at least one secret for SSO RBAC service account: %s", serviceAccount.GetName())
}
secret, err := s.cache.GetSecret(ctx, serviceAccount.GetNamespace(), serviceAccount.Secrets[0].Name)
secretName := secrets.SecretName(serviceAccount)
secret, err := s.cache.GetSecret(ctx, serviceAccount.GetNamespace(), secretName)
if err != nil {
return "", fmt.Errorf("failed to get service account secret: %w", err)
}
Expand Down
7 changes: 3 additions & 4 deletions server/auth/webhook/interceptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"net/http"
"strings"

"github.com/argoproj/argo-workflows/v3/util/secrets"

log "github.com/sirupsen/logrus"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -84,10 +86,7 @@ func addWebhookAuthorization(r *http.Request, kube kubernetes.Interface) error {
if err != nil {
return fmt.Errorf("failed to get service account \"%s\": %w", serviceAccountName, err)
}
if len(serviceAccount.Secrets) == 0 {
return fmt.Errorf("failed to get secret for service account \"%s\": no secrets", serviceAccountName)
}
tokenSecret, err := secretsInterface.Get(ctx, serviceAccount.Secrets[0].Name, metav1.GetOptions{})
tokenSecret, err := secretsInterface.Get(ctx, secrets.SecretName(serviceAccount), metav1.GetOptions{})
if err != nil {
return fmt.Errorf("failed to get token secret \"%s\": %w", tokenSecret, err)
}
Expand Down
6 changes: 4 additions & 2 deletions test/e2e/argo_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
"testing"
"time"

"github.com/argoproj/argo-workflows/v3/util/secrets"

"github.com/gavv/httpexpect/v2"
log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -445,7 +447,7 @@ func (s *ArgoServerSuite) TestPermission() {
s.Run("GetGoodSAToken", func() {
sAccount, err := s.KubeClient.CoreV1().ServiceAccounts(nsName).Get(ctx, goodSaName, metav1.GetOptions{})
if assert.NoError(s.T(), err) {
secretName := sAccount.Secrets[0].Name
secretName := secrets.SecretName(sAccount)
secret, err := s.KubeClient.CoreV1().Secrets(nsName).Get(ctx, secretName, metav1.GetOptions{})
assert.NoError(s.T(), err)
goodToken = string(secret.Data["token"])
Expand All @@ -457,7 +459,7 @@ func (s *ArgoServerSuite) TestPermission() {
s.Run("GetBadSAToken", func() {
sAccount, err := s.KubeClient.CoreV1().ServiceAccounts(nsName).Get(ctx, badSaName, metav1.GetOptions{})
assert.NoError(s.T(), err)
secretName := sAccount.Secrets[0].Name
secretName := secrets.SecretName(sAccount)
secret, err := s.KubeClient.CoreV1().Secrets(nsName).Get(ctx, secretName, metav1.GetOptions{})
assert.NoError(s.T(), err)
badToken = string(secret.Data["token"])
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
generateName: template-executor-
name: template-executor
spec:
entrypoint: main
templates:
Expand Down
15 changes: 15 additions & 0 deletions util/secrets/secret_name.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package secrets

import (
"fmt"

corev1 "k8s.io/api/core/v1"
)

func SecretName(serviceAccount *corev1.ServiceAccount) string {
secretName := fmt.Sprintf("%s.service-account-token", serviceAccount.Name)
if len(serviceAccount.Secrets) > 0 {
secretName = serviceAccount.Secrets[0].Name
}
return secretName
}
19 changes: 19 additions & 0 deletions util/secrets/secret_name_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package secrets

import (
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
)

func TestSecretName(t *testing.T) {
sa := corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{Name: "sa-name"},
}
assert.Equal(t, "sa-name.service-account-token", SecretName(&sa))
sa.Secrets = []corev1.ObjectReference{{Name: "existing-secret"}}
assert.Equal(t, "existing-secret", SecretName(&sa))
}
7 changes: 3 additions & 4 deletions workflow/controller/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (
"sync"
"time"

"github.com/argoproj/argo-workflows/v3/util/secrets"

"github.com/antonmedv/expr"
"github.com/argoproj/pkg/humanize"
argokubeerr "github.com/argoproj/pkg/kube/errors"
Expand Down Expand Up @@ -3686,10 +3688,7 @@ func (woc *wfOperationCtx) getServiceAccountTokenName(ctx context.Context, name
if err != nil {
return "", err
}
if len(account.Secrets) == 0 {
return "", fmt.Errorf("service account %s/%s does not have any secrets", account.Namespace, account.Name)
}
return account.Secrets[0].Name, nil
return secrets.SecretName(account), nil
Comment on lines -3690 to +3691
Copy link
Member

@jessesuen jessesuen Sep 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are breaking backwards compatibility here.

Users of v1.23 and below, are not conditioned to generate ServiceAccount secrets yet and so having the assumption that there exists the secret by naming convention is going to break a lot of.

I think the implementation needs to have both implementations and preserve the older v1.23 assumed behavior for backward compatibility

EDIT: Nevermind, I just saw the implementation of SecretName and see that it is doing just that.

}

// setWfPodNamesAnnotation sets an annotation on a workflow with the pod naming
Expand Down