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

Provide serviceAccountRef name/namespace on lifecycle-image configmap #837

Merged
merged 1 commit into from
Sep 28, 2021
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
3 changes: 1 addition & 2 deletions cmd/controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ var (
rebaseImage = flag.String("rebase-image", os.Getenv("REBASE_IMAGE"), "The image used to perform rebases")
completionImage = flag.String("completion-image", os.Getenv("COMPLETION_IMAGE"), "The image used to finish a build")
completionWindowsImage = flag.String("completion-windows-image", os.Getenv("COMPLETION_WINDOWS_IMAGE"), "The image used to finish a build on windows")
lifecycleImage = flag.String("lifecycle-image", os.Getenv("LIFECYCLE_IMAGE"), "The image used to provide lifecycle binaries")
)

func main() {
Expand Down Expand Up @@ -161,7 +160,7 @@ func main() {
log.Fatalf("could not create empty keychain %s", err)
}

lifecycleProvider := config.NewLifecycleProvider(*lifecycleImage, &registry.Client{}, kpackKeychain)
lifecycleProvider := config.NewLifecycleProvider(&registry.Client{}, keychainFactory)
configMapWatcher.Watch(config.LifecycleConfigName, lifecycleProvider.UpdateImage)

builderCreator := &cnb.RemoteBuilderCreator{
Expand Down
5 changes: 0 additions & 5 deletions config/controller.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,6 @@ spec:
configMapKeyRef:
name: completion-windows-image
key: image
- name: LIFECYCLE_IMAGE
valueFrom:
configMapKeyRef:
name: lifecycle-image
key: image
resources:
requests:
cpu: 20m
Expand Down
51 changes: 29 additions & 22 deletions pkg/config/lifecycle_provider.go
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
package config

import (
"context"
"sync/atomic"

"github.com/google/go-containerregistry/pkg/authn"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"

"github.com/pivotal/kpack/pkg/registry"
)

const (
LifecycleConfigName = "lifecycle-image"
LifecycleConfigKey = "image"
LifecycleConfigName = "lifecycle-image"
LifecycleConfigKey = "image"
ServiceAccountNameKey = "serviceAccountRef.name"
ServiceAccountNamespaceKey = "serviceAccountRef.namespace"
)

type RegistryClient interface {
Expand All @@ -24,43 +29,45 @@ type lifecycleData struct {
}

type LifecycleProvider struct {
RegistryClient RegistryClient
Keychain authn.Keychain
lifecycleData atomic.Value
handlers []func()
registryClient RegistryClient
keychainFactory registry.KeychainFactory
lifecycleData atomic.Value
handlers []func()
}

func NewLifecycleProvider(lifecycleImageRef string, client RegistryClient, keychain authn.Keychain) *LifecycleProvider {
p := &LifecycleProvider{
RegistryClient: client,
Keychain: keychain,
func NewLifecycleProvider(client RegistryClient, keychainFactory registry.KeychainFactory) *LifecycleProvider {
return &LifecycleProvider{
registryClient: client,
keychainFactory: keychainFactory,
}

data := &lifecycleData{}

p.fetchImage(lifecycleImageRef, data)

p.lifecycleData.Store(data)
return p
}

func (l *LifecycleProvider) UpdateImage(cm *corev1.ConfigMap) {
data, shouldCallHandlers := l.updateImage(cm)
data, shouldCallHandlers := l.updateImage(context.Background(), cm)
if shouldCallHandlers {
l.callHandlers()
}
l.lifecycleData.Store(data)
}

func (l *LifecycleProvider) updateImage(cm *corev1.ConfigMap) (*lifecycleData, bool) {
func (l *LifecycleProvider) updateImage(ctx context.Context, cm *corev1.ConfigMap) (*lifecycleData, bool) {
data := &lifecycleData{}
imageRef, ok := cm.Data[LifecycleConfigKey]
if !ok {
data.err = errors.Errorf("%s config invalid", LifecycleConfigName)
return data, true
}

l.fetchImage(imageRef, data)
keychain, err := l.keychainFactory.KeychainForSecretRef(ctx, registry.SecretRef{
ServiceAccount: cm.Data[ServiceAccountNameKey],
Namespace: cm.Data[ServiceAccountNamespaceKey],
})
if err != nil {
data.err = errors.Wrapf(err, "fetching keychain to read lifecycle")
return data, true
}

l.fetchImage(keychain, imageRef, data)
if data.err != nil {
return data, true
}
Expand All @@ -85,8 +92,8 @@ func (l *LifecycleProvider) AddEventHandler(handler func()) {
l.handlers = append(l.handlers, handler)
}

func (l *LifecycleProvider) fetchImage(imageRef string, data *lifecycleData) {
img, _, err := l.RegistryClient.Fetch(l.Keychain, imageRef)
func (l *LifecycleProvider) fetchImage(keychain authn.Keychain, imageRef string, data *lifecycleData) {
img, _, err := l.registryClient.Fetch(keychain, imageRef)
if err != nil {
data.err = errors.Wrap(err, "failed to fetch lifecycle image")
return
Expand Down
52 changes: 23 additions & 29 deletions pkg/config/lifecycle_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"

"github.com/pivotal/kpack/pkg/registry"
"github.com/pivotal/kpack/pkg/registry/registryfakes"
)

Expand All @@ -19,58 +20,51 @@ func TestProvider(t *testing.T) {

func testProvider(t *testing.T, when spec.G, it spec.S) {
var (
client = registryfakes.NewFakeClient()
keychain = authn.NewMultiKeychain(authn.DefaultKeychain)
lifecycleImgRef = "some-image"
newLifecycleImgRef = "some-other-image"
lifecycleImg v1.Image
newLifecycleImg v1.Image
callBack *fakeCallback
err error
p *LifecycleProvider
client = registryfakes.NewFakeClient()
keychain = authn.NewMultiKeychain(authn.DefaultKeychain)
lifecycleImgRef = "some-image"
lifecycleImg v1.Image
callBack *fakeCallback
err error
keychainFactory = &registryfakes.FakeKeychainFactory{}
p *LifecycleProvider
)

it.Before(func() {
lifecycleImg, err = random.Image(10, int64(1))
require.NoError(t, err)
newLifecycleImg, err = random.Image(10, int64(1))
require.NoError(t, err)

keychainFactory.AddKeychainForSecretRef(t, registry.SecretRef{Namespace: "some-service-account-namespace", ServiceAccount: "some-service-account"}, keychain)
client.AddImage(lifecycleImgRef, lifecycleImg, keychain)
client.AddImage(newLifecycleImgRef, newLifecycleImg, keychain)

p = NewLifecycleProvider(lifecycleImgRef, client, keychain)
p = NewLifecycleProvider(client, keychainFactory)
callBack = &fakeCallback{}
p.AddEventHandler(callBack.callBack)
})

it("is seeded with a lifecycle image", func() {
img, err := p.GetImage()
require.NoError(t, err)
require.Equal(t, lifecycleImg, img)
})

it("sets and gets the image from the ConfigMap and calls handlers", func() {
cfg := &corev1.ConfigMap{
Data: map[string]string{"image": "some-other-image"},
Data: map[string]string{"image": "some-image", "serviceAccountRef.name": "some-service-account", "serviceAccountRef.namespace": "some-service-account-namespace"},
}

p.UpdateImage(cfg)
img, err := p.GetImage()
require.NoError(t, err)
require.Equal(t, newLifecycleImg, img)
require.True(t, callBack.called)
require.Equal(t, lifecycleImg, img)
require.Equal(t, callBack.called, 1)
})

it("does not call handlers when the lifecycle image has not changed", func() {
cfg := &corev1.ConfigMap{
Data: map[string]string{"image": "some-image"},
Data: map[string]string{"image": "some-image", "serviceAccountRef.name": "some-service-account", "serviceAccountRef.namespace": "some-service-account-namespace"},
}

p.UpdateImage(cfg)
p.UpdateImage(cfg)
img, err := p.GetImage()
require.NoError(t, err)
require.Equal(t, lifecycleImg, img)
require.False(t, callBack.called)
require.Equal(t, callBack.called, 1)
})

it("updates after an error", func() {
Expand All @@ -82,12 +76,12 @@ func testProvider(t *testing.T, when spec.G, it spec.S) {
require.Error(t, err)

cfg = &corev1.ConfigMap{
Data: map[string]string{"image": "some-other-image"},
Data: map[string]string{"image": "some-image", "serviceAccountRef.name": "some-service-account", "serviceAccountRef.namespace": "some-service-account-namespace"},
}
p.UpdateImage(cfg)
img, err := p.GetImage()
require.NoError(t, err)
require.Equal(t, newLifecycleImg, img)
require.Equal(t, lifecycleImg, img)
})

it("errors when the image key is invalid and calls handlers", func() {
Expand All @@ -98,7 +92,7 @@ func testProvider(t *testing.T, when spec.G, it spec.S) {
p.UpdateImage(cfg)
_, err := p.GetImage()
require.EqualError(t, err, "lifecycle-image config invalid")
require.True(t, callBack.called)
require.Equal(t, callBack.called, 1)
})

it("errors when it has not loaded an image yet", func() {
Expand All @@ -109,9 +103,9 @@ func testProvider(t *testing.T, when spec.G, it spec.S) {
}

type fakeCallback struct {
called bool
called int
}

func (cb *fakeCallback) callBack() {
cb.called = true
cb.called++
}