Skip to content

Commit

Permalink
Pass app env vars into the kpack image
Browse files Browse the repository at this point in the history 
* App environment consists of entries in the app env secret and
  `VCAP_SERVICES` that is built out of app service bindings
* Introduce a `env.Builder` to build the app environment which is used
  by the process controller and the build controller
* The `env.Builder` adds the `VCAP_SERVICES` entry into the app env
  secret under the hood
* If the app has no env secret name set, the builder would be a noop and
  no env would be passed to the process/build. We assume that in this
  case the developer does not care about the environment
* If the app has an env secret name set, but the secret does not exist,
  we return an error as we assume that the secret should have been already
  created

TODO: The controllers account now needs permissions to patch secrets

Issue: #774

Co-authored-by: Kieron Browne <kbrowne@vmware.com>
Co-authored-by: Danail Branekov <danailster@gmail.com>
Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>
  • Loading branch information
@danail-branekov @georgethebeatle
3 people committed Apr 5, 2022
1 parent 6b21836 commit c85daf6
Show file tree
Hide file tree
Showing 12 changed files with 1,056 additions and 602 deletions.
99 changes: 60 additions & 39 deletions controllers/controllers/workloads/cfbuild_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ type CFBuildReconciler struct {
ControllerConfig *config.ControllerConfig
RegistryAuthFetcher RegistryAuthFetcher
ImageProcessFetcher ImageProcessFetcher
EnvBuilder EnvBuilder
}

//+kubebuilder:rbac:groups=workloads.cloudfoundry.org,resources=cfbuilds,verbs=get;list;watch;create;update;patch;delete
Expand Down Expand Up @@ -154,20 +155,13 @@ func (r *CFBuildReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
// Scenario: CFBuild newly created and all status conditions are unknown, it
// Creates a KpackImage resource to trigger staging.
// Updates status on CFBuild -> sets staging to True.
var appServiceBindings []servicesv1alpha1.CFServiceBinding
appServiceBindings, err = r.getAppServiceBindings(ctx, cfApp.Name, cfApp.Namespace)
if err != nil {
r.Log.Error(err, "Error when listing CFServiceBindings")
return ctrl.Result{}, err
}

err = r.ensureKpackImageRequirements(ctx, cfPackage)
if err != nil {
r.Log.Info("Kpack image requirements for CFPackage are not met", "guid", cfPackage.Name, "reason", err)
return ctrl.Result{}, err
}

err = r.createKpackImageAndUpdateStatus(ctx, cfBuild, cfApp, cfPackage, appServiceBindings)
err = r.createKpackImageAndUpdateStatus(ctx, cfBuild, cfApp, cfPackage)
if err != nil {
return ctrl.Result{}, err
}
Expand Down Expand Up @@ -228,24 +222,7 @@ func (r *CFBuildReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
return ctrl.Result{}, nil
}

func (r *CFBuildReconciler) getAppServiceBindings(ctx context.Context, appGUID string, namespace string) ([]servicesv1alpha1.CFServiceBinding, error) {
serviceBindings := &servicesv1alpha1.CFServiceBindingList{}
err := r.Client.List(ctx, serviceBindings,
client.InNamespace(namespace),
client.MatchingFields{shared.IndexServiceBindingAppGUID: appGUID},
)
if err != nil {
return nil, fmt.Errorf("error listing CFServiceBindings: %w", err)
}

if len(serviceBindings.Items) == 0 {
return nil, nil
}

return serviceBindings.Items, nil
}

func (r *CFBuildReconciler) createKpackImageAndUpdateStatus(ctx context.Context, cfBuild *workloadsv1alpha1.CFBuild, cfApp *workloadsv1alpha1.CFApp, cfPackage *workloadsv1alpha1.CFPackage, serviceBindings []servicesv1alpha1.CFServiceBinding) error {
func (r *CFBuildReconciler) createKpackImageAndUpdateStatus(ctx context.Context, cfBuild *workloadsv1alpha1.CFBuild, cfApp *workloadsv1alpha1.CFApp, cfPackage *workloadsv1alpha1.CFPackage) error {
serviceAccountName := kpackServiceAccount
kpackImageTag := path.Join(r.ControllerConfig.KpackImageTag, cfBuild.Name)
kpackImageName := cfBuild.Name
Expand Down Expand Up @@ -278,21 +255,20 @@ func (r *CFBuildReconciler) createKpackImageAndUpdateStatus(ctx context.Context,
},
},
}
for _, serviceBinding := range serviceBindings {
if serviceBinding.Status.Binding.Name != "" {
objRef := corev1.ObjectReference{
Kind: "Secret",
Name: serviceBinding.Status.Binding.Name,
APIVersion: "v1",
}
desiredKpackImage.Spec.Build.Services = append(desiredKpackImage.Spec.Build.Services, objRef)
} else {
r.Log.Info("binding secret name is empty")
return errors.New("binding secret name is empty")
}

buildServices, err := r.prepareBuildServices(ctx, kpackImageNamespace, cfApp.Name)
if err != nil {
return err
}
desiredKpackImage.Spec.Build.Services = buildServices

err := controllerutil.SetOwnerReference(cfBuild, &desiredKpackImage, r.Scheme)
imageEnvironment, err := r.prepareEnvironment(ctx, cfApp)
if err != nil {
return err
}
desiredKpackImage.Spec.Build.Env = imageEnvironment

err = controllerutil.SetOwnerReference(cfBuild, &desiredKpackImage, r.Scheme)
if err != nil {
r.Log.Error(err, "failed to set OwnerRef on Kpack Image")
return err
Expand All @@ -314,6 +290,51 @@ func (r *CFBuildReconciler) createKpackImageAndUpdateStatus(ctx context.Context,
return nil
}

func (r *CFBuildReconciler) prepareBuildServices(ctx context.Context, namespace, appGUID string) (buildv1alpha2.Services, error) {
serviceBindingsList := &servicesv1alpha1.CFServiceBindingList{}
err := r.Client.List(ctx, serviceBindingsList,
client.InNamespace(namespace),
client.MatchingFields{shared.IndexServiceBindingAppGUID: appGUID},
)
if err != nil {
return nil, fmt.Errorf("error listing CFServiceBindings: %w", err)
}

buildServices := buildv1alpha2.Services{}
for _, serviceBinding := range serviceBindingsList.Items {
if serviceBinding.Status.Binding.Name != "" {
objRef := corev1.ObjectReference{
Kind: "Secret",
Name: serviceBinding.Status.Binding.Name,
APIVersion: "v1",
}
buildServices = append(buildServices, objRef)
} else {
r.Log.Info("binding secret name is empty")
return nil, errors.New("binding secret name is empty")
}
}
return buildServices, nil
}

func (r *CFBuildReconciler) prepareEnvironment(ctx context.Context, cfApp *workloadsv1alpha1.CFApp) ([]corev1.EnvVar, error) {
env, err := r.EnvBuilder.BuildEnv(ctx, cfApp)
if err != nil {
r.Log.Error(err, "failed building environment")
return nil, err
}

imageEnvironment := []corev1.EnvVar{}
for k, v := range env {
imageEnvironment = append(imageEnvironment, corev1.EnvVar{
Name: k,
Value: v,
})
}

return imageEnvironment, nil
}

func (r *CFBuildReconciler) createKpackImageIfNotExists(ctx context.Context, desiredKpackImage buildv1alpha2.Image) error {
var foundKpackImage buildv1alpha2.Image
err := r.Client.Get(ctx, types.NamespacedName{Name: desiredKpackImage.Name, Namespace: desiredKpackImage.Namespace}, &foundKpackImage)
Expand Down
49 changes: 37 additions & 12 deletions controllers/controllers/workloads/cfbuild_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ var _ = Describe("CFBuildReconciler", func() {

fakeRegistryAuthFetcher *fake.RegistryAuthFetcher
fakeImageProcessFetcher *fake.ImageProcessFetcher
fakeEnvBuilder *fake.EnvBuilder

cfAppGUID string
cfPackageGUID string
Expand Down Expand Up @@ -94,7 +95,7 @@ var _ = Describe("CFBuildReconciler", func() {
kpackServiceAccount = BuildServiceAccount(kpackServiceAccountName, defaultNamespace, kpackRegistrySecretName)
kpackServiceAccountError = nil

fakeClient.GetStub = func(_ context.Context, _ types.NamespacedName, obj client.Object) error {
fakeClient.GetStub = func(_ context.Context, namespacedName types.NamespacedName, obj client.Object) error {
// cast obj to find its kind
switch obj := obj.(type) {
case *workloadsv1alpha1.CFBuild:
Expand All @@ -120,15 +121,13 @@ var _ = Describe("CFBuildReconciler", func() {
}
}

// Configure mock status update to succeed
fakeStatusWriter = &fake.StatusWriter{}
fakeStatusWriter = new(fake.StatusWriter)
fakeClient.StatusReturns(fakeStatusWriter)

// Configure a fake RegistryAuthFetcher
fakeRegistryAuthFetcher = &fake.RegistryAuthFetcher{}

// Configure a fake ImageProcessFetcher
fakeImageProcessFetcher = &fake.ImageProcessFetcher{}
fakeRegistryAuthFetcher = new(fake.RegistryAuthFetcher)
fakeImageProcessFetcher = new(fake.ImageProcessFetcher)
fakeEnvBuilder = new(fake.EnvBuilder)
fakeEnvBuilder.BuildEnvReturns(map[string]string{"foo": "var"}, nil)

// configure a CFBuildReconciler with the client
Expect(workloadsv1alpha1.AddToScheme(scheme.Scheme)).To(Succeed())
Expand All @@ -140,6 +139,7 @@ var _ = Describe("CFBuildReconciler", func() {
ControllerConfig: &config.ControllerConfig{KpackImageTag: "image/registry/tag"},
RegistryAuthFetcher: fakeRegistryAuthFetcher.Spy,
ImageProcessFetcher: fakeImageProcessFetcher.Spy,
EnvBuilder: fakeEnvBuilder,
}
req = ctrl.Request{
NamespacedName: types.NamespacedName{
Expand All @@ -166,8 +166,8 @@ var _ = Describe("CFBuildReconciler", func() {

It("should create kpack image with the same GUID as the CF Build", func() {
Expect(fakeClient.CreateCallCount()).To(Equal(1), "fakeClient Create was not called 1 time")
_, kpackImage, _ := fakeClient.CreateArgsForCall(0)
Expect(kpackImage.GetName()).To(Equal(cfBuildGUID))
_, actualKpackImage, _ := fakeClient.CreateArgsForCall(0)
Expect(actualKpackImage.GetName()).To(Equal(cfBuildGUID))
})

It("should update the status conditions on CFBuild", func() {
Expand All @@ -176,15 +176,30 @@ var _ = Describe("CFBuildReconciler", func() {

It("should create kpack Image with CFBuild owner reference", func() {
Expect(fakeClient.CreateCallCount()).To(Equal(1), "fakeClient Create was not called 1 time")
_, kpackImage, _ := fakeClient.CreateArgsForCall(0)
ownerRefs := kpackImage.GetOwnerReferences()
_, actualKpackImage, _ := fakeClient.CreateArgsForCall(0)
ownerRefs := actualKpackImage.GetOwnerReferences()
Expect(ownerRefs).To(ConsistOf(metav1.OwnerReference{
UID: cfBuild.UID,
Kind: cfBuild.Kind,
APIVersion: cfBuild.APIVersion,
Name: cfBuild.Name,
}))
})

It("sets the env vars on the kpack image", func() {
Expect(fakeClient.CreateCallCount()).To(Equal(1))
_, obj, _ := fakeClient.CreateArgsForCall(0)
actualKpackImage, ok := obj.(*buildv1alpha2.Image)
Expect(ok).To(BeTrue(), "create wasn't passed a kpackImage")

Expect(fakeEnvBuilder.BuildEnvCallCount()).To(Equal(1))
_, actualApp := fakeEnvBuilder.BuildEnvArgsForCall(0)
Expect(actualApp).To(Equal(cfApp))

Expect(actualKpackImage.Spec.Build.Env).To(HaveLen(1))
Expect(actualKpackImage.Spec.Build.Env[0].Name).To(Equal("foo"))
Expect(actualKpackImage.Spec.Build.Env[0].Value).To(Equal("var"))
})
})

When("on unhappy path", func() {
Expand Down Expand Up @@ -271,6 +286,16 @@ var _ = Describe("CFBuildReconciler", func() {
Expect(reconcileErr).To(HaveOccurred())
})
})

When("building environment fails", func() {
BeforeEach(func() {
fakeEnvBuilder.BuildEnvReturns(nil, errors.New("boom"))
})

It("should return an error", func() {
Expect(reconcileErr).To(HaveOccurred())
})
})
})
})

Expand Down
Loading

0 comments on commit c85daf6

Please sign in to comment.