Skip to content

Commit

Permalink
ServiceAccount without ImagePullSecrets should not fail to fetch
Browse files Browse the repository at this point in the history
While `ImagePullSecret`s would still be needed for any private
registry, not having any on the specified service account shouldn't
cause a failure to even try to fetch the image.

Fixes tektoncd#584
  • Loading branch information
abayer committed Mar 5, 2019
1 parent 3328a04 commit db60cf7
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 33 deletions.
40 changes: 7 additions & 33 deletions pkg/reconciler/v1alpha1/taskrun/entrypoint/entrypoint.go
Expand Up @@ -27,11 +27,10 @@ import (
"github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/google"
"github.com/google/go-containerregistry/pkg/v1/remote"
lru "github.com/hashicorp/golang-lru"
"github.com/hashicorp/golang-lru"
buildv1alpha1 "github.com/knative/build/pkg/apis/build/v1alpha1"
"go.uber.org/zap"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"

"github.com/knative/build/pkg/apis/build/v1alpha1"
Expand Down Expand Up @@ -187,38 +186,13 @@ func getRemoteImage(image string, kubeclient kubernetes.Interface, build *buildv
if err != nil {
return nil, fmt.Errorf("Failed to parse image %s: %v", image, err)
}
var kc authn.Keychain
serviceAccountName := build.Spec.ServiceAccountName
if serviceAccountName != "" && serviceAccountName != "default" {
sa, err := kubeclient.CoreV1().ServiceAccounts(build.Namespace).Get(serviceAccountName, metav1.GetOptions{})
if err != nil {
return nil, fmt.Errorf("Failed to get service account %s: %v", serviceAccountName, err)
}
if len(sa.ImagePullSecrets) == 0 {
return nil, fmt.Errorf("No ImagePullSecret for service account %s: %v", serviceAccountName, err)
}
for _, secret := range sa.ImagePullSecrets {
_, err = kubeclient.CoreV1().Secrets(build.Namespace).Get(secret.Name, metav1.GetOptions{})
if err != nil {
return nil, fmt.Errorf("Failed to get ImagePullSecret for service account %s: %v", serviceAccountName, err)
}

kc, err = k8schain.New(kubeclient, k8schain.Options{
ImagePullSecrets: []string{secret.Name},
Namespace: build.Namespace,
})
if err != nil {
return nil, fmt.Errorf("Failed to create k8schain: %v", err)
}
}
} else {
kc, err = k8schain.New(kubeclient, k8schain.Options{
Namespace: build.Namespace,
ServiceAccountName: "default",
})
if err != nil {
return nil, fmt.Errorf("Failed to create k8schain: %v", err)
}
kc, err := k8schain.New(kubeclient, k8schain.Options{
Namespace: build.Namespace,
ServiceAccountName: build.Spec.ServiceAccountName,
})
if err != nil {
return nil, fmt.Errorf("Failed to create k8schain: %v", err)
}

// this will first try to authenticate using the k8schain,
Expand Down
67 changes: 67 additions & 0 deletions pkg/reconciler/v1alpha1/taskrun/entrypoint/entrypoint_test.go
Expand Up @@ -260,6 +260,73 @@ func TestGetRemoteEntrypoint(t *testing.T) {
}
}

func TestGetRemoteEntrypointWithNonDefaultSA(t *testing.T) {
expectedEntrypoint := []string{"/bin/expected", "entrypoint"}
img := getImage(t, &v1.ConfigFile{
Config: v1.Config{
Entrypoint: expectedEntrypoint,
},
})
expectedRepo := "image"
digetsSha := getDigestAsString(img)
configPath := fmt.Sprintf("/v2/%s/blobs/%s", expectedRepo, mustConfigName(t, img))
manifestPath := fmt.Sprintf("/v2/%s/manifests/%s", expectedRepo, digetsSha)

server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/v2/":
w.WriteHeader(http.StatusOK)
case configPath:
if r.Method != http.MethodGet {
t.Errorf("Method; got %v, want %v", r.Method, http.MethodGet)
}
w.Write(mustRawConfigFile(t, img))
case manifestPath:
if r.Method != http.MethodGet {
t.Errorf("Method; got %v, want %v", r.Method, http.MethodGet)
}
w.Write(mustRawManifest(t, img))
default:
t.Fatalf("Unexpected path: %v", r.URL.Path)
}
}))
defer server.Close()
image := path.Join(strings.TrimPrefix(server.URL, "http://"), expectedRepo)
finalDigest := image + "@" + digetsSha

entrypointCache, err := NewCache()
if err != nil {
t.Fatalf("couldn't create new entrypoint cache: %v", err)
}
build := &v1alpha1.Build{
ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
Name: "build",
},
Spec: v1alpha1.BuildSpec{
ServiceAccountName: "some-other-sa",
Steps: []corev1.Container{{
Image: "ubuntu",
Command: []string{"echo"},
Args: []string{"hello"},
}},
},
}
c := fakekubeclientset.NewSimpleClientset(&corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: "some-other-sa",
Namespace: "foo",
},
})
ep, err := GetRemoteEntrypoint(entrypointCache, finalDigest, c, build)
if err != nil {
t.Errorf("couldn't get entrypoint remote: %v", err)
}
if !reflect.DeepEqual(ep, expectedEntrypoint) {
t.Errorf("entrypoints do not match: %s should be %s", ep[0], expectedEntrypoint)
}
}

func TestEntrypointCacheLRU(t *testing.T) {
entrypoint := []string{"/bin/expected", "entrypoint"}
entrypointCache, err := NewCache()
Expand Down

0 comments on commit db60cf7

Please sign in to comment.