Merge pull request #2305 from thedadams/secrets-first

Refactor DeploySpec so that secrets are first

pkg/controller/appdefinition/acorn.go

@@ -20,38 +20,10 @@ import (
 	"golang.org/x/exp/slices"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	kclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-func addAcorns(req router.Request, appInstance *v1.AppInstance, tag name.Reference, pullSecrets *PullSecrets, resp router.Response) error {
-	for _, acorn := range toAcorns(appInstance, tag, pullSecrets) {
-		var devSession v1.DevSessionInstance
-		err := req.Get(&devSession, acorn.Namespace, acorn.Name)
-		if err == nil {
-			// Don't update app in dev mode
-			acorn.Annotations[apply.AnnotationUpdate] = "false"
-		} else if !apierrors.IsNotFound(err) {
-			return err
-		}
-		var existingApp v1.AppInstance
-		err = req.Get(&existingApp, acorn.Namespace, acorn.Name)
-		if err == nil {
-			if slices.Contains(existingApp.Finalizers, jobs.DestroyJobFinalizer) {
-				acorn.Annotations[apply.AnnotationPrune] = "false"
-			}
-		} else if !apierrors.IsNotFound(err) {
-			return err
-		}
-
-		if strings.Count(acorn.Labels[labels.AcornPublicName], ".") > 10 {
-			return fmt.Errorf("max limit of 10 nested acorns exceeded")
-		}
-
-		resp.Objects(acorn)
-	}
-	return nil
-}
-
-func toAcorns(appInstance *v1.AppInstance, tag name.Reference, pullSecrets *PullSecrets) (result []*v1.AppInstance) {
+func toAcorns(req router.Request, appInstance *v1.AppInstance, tag name.Reference, pullSecrets *PullSecrets) (result []kclient.Object, _ error) {
 	for _, entry := range typed.Sorted(appInstance.Status.AppSpec.Acorns) {
 		acornName, acorn := entry.Key, entry.Value
 		if ports.IsLinked(appInstance, acornName) {
@@ -64,7 +36,8 @@ func toAcorns(appInstance *v1.AppInstance, tag name.Reference, pullSecrets *Pull
 		if ports.IsLinked(appInstance, serviceName) || service.Image == "" {
 			continue
 		}
-		result = append(result, toAcorn(appInstance, tag, pullSecrets, serviceName, appInstance.Status.AppStatus.Services[serviceName].ConfigHash, v1.Acorn{
+
+		acorn := toAcorn(appInstance, tag, pullSecrets, serviceName, appInstance.Status.AppStatus.Services[serviceName].ConfigHash, v1.Acorn{
 			Labels:              service.Labels,
 			Annotations:         service.Annotations,
 			Image:               service.Image,
@@ -78,9 +51,33 @@ func toAcorns(appInstance *v1.AppInstance, tag name.Reference, pullSecrets *Pull
 			AutoUpgradeInterval: service.AutoUpgradeInterval,
 			Memory:              service.Memory,
 			ComputeClasses:      service.ComputeClasses,
-		}))
+		})
+
+		var devSession v1.DevSessionInstance
+		err := req.Get(&devSession, acorn.Namespace, acorn.Name)
+		if err == nil {
+			// Don't update app in dev mode
+			acorn.Annotations[apply.AnnotationUpdate] = "false"
+		} else if !apierrors.IsNotFound(err) {
+			return nil, err
+		}
+		var existingApp v1.AppInstance
+		err = req.Get(&existingApp, acorn.Namespace, acorn.Name)
+		if err == nil {
+			if slices.Contains(existingApp.Finalizers, jobs.DestroyJobFinalizer) {
+				acorn.Annotations[apply.AnnotationPrune] = "false"
+			}
+		} else if !apierrors.IsNotFound(err) {
+			return nil, err
+		}
+
+		if strings.Count(acorn.Labels[labels.AcornPublicName], ".") > 10 {
+			return nil, fmt.Errorf("max limit of 10 nested acorns exceeded")
+		}
+
+		result = append(result, acorn)
 	}
-	return result
+	return result, nil
 }
 
 func scopeSecrets(app *v1.AppInstance, bindings v1.SecretBindings) (result v1.SecretBindings) {

pkg/controller/appdefinition/deploy.go

@@ -25,6 +25,7 @@ import (
 	"github.com/acorn-io/runtime/pkg/publicname"
 	"github.com/acorn-io/runtime/pkg/ref"
 	"github.com/acorn-io/runtime/pkg/secrets"
+	"github.com/acorn-io/runtime/pkg/services"
 	"github.com/acorn-io/runtime/pkg/system"
 	"github.com/acorn-io/runtime/pkg/volume"
 	"github.com/acorn-io/z"
@@ -61,6 +62,7 @@ func DeploySpec(req router.Request, resp router.Response) (err error) {
 	appInstance := req.Object.(*v1.AppInstance)
 	status := condition.Setter(appInstance, resp, v1.AppInstanceConditionDefined)
 	interpolator := secrets.NewInterpolator(req.Ctx, req.Client, appInstance)
+	var result []kclient.Object
 
 	defer func() {
 		if err == nil {
@@ -90,62 +92,44 @@ func DeploySpec(req router.Request, resp router.Response) (err error) {
 		return err
 	}
 
-	if err := addDeployments(req, appInstance, tag, pullSecrets, interpolator, resp); err != nil {
+	if objs, err := ToDeployments(req, appInstance, tag, pullSecrets, interpolator); err != nil {
 		return err
+	} else if len(objs) > 0 {
+		result = append(result, objs...)
 	}
-	if err := addRouters(appInstance, resp); err != nil {
+	if objs, err := toRouters(appInstance); err != nil {
 		return err
+	} else {
+		result = append(result, objs...)
 	}
-	if err := addJobs(req, appInstance, tag, pullSecrets, interpolator, resp); err != nil {
+	if objs, err := toJobs(req, appInstance, pullSecrets, tag, interpolator); err != nil {
 		return err
+	} else {
+		result = append(result, objs...)
 	}
-	if err := addServices(req, appInstance, interpolator, resp); err != nil {
+	if objs, err := services.ToAcornServices(req.Ctx, req.Client, interpolator, appInstance); err != nil {
 		return err
+	} else {
+		result = append(result, objs...)
 	}
-	if err := addPVCs(req, appInstance, resp); err != nil {
+	if objs, err := toPVCs(req, appInstance); err != nil {
 		return err
+	} else {
+		result = append(result, objs...)
 	}
-	if err := addAcorns(req, appInstance, tag, pullSecrets, resp); err != nil {
+	if objs, err := toAcorns(req, appInstance, tag, pullSecrets); err != nil {
 		return err
+	} else {
+		result = append(result, objs...)
 	}
 
+	// Secrets go in first so that they are created/updated before things that depend on them.
 	resp.Objects(pullSecrets.Objects()...)
 	resp.Objects(interpolator.Objects()...)
+	resp.Objects(result...)
 	return pullSecrets.Err()
 }
 
-func addDeployments(req router.Request, appInstance *v1.AppInstance, tag name.Reference, pullSecrets *PullSecrets, secrets *secrets.Interpolator, resp router.Response) error {
-	deps, err := ToDeployments(req, appInstance, tag, pullSecrets, secrets)
-	if err != nil {
-		return err
-	}
-
-outer:
-	for _, obj := range deps {
-		if dep, ok := obj.(*appsv1.Deployment); ok {
-			for _, v := range dep.Spec.Template.Spec.Volumes {
-				if v.Name == string(appInstance.UID) {
-					deps = append(deps, &corev1.ConfigMap{
-						ObjectMeta: metav1.ObjectMeta{
-							Name:      string(appInstance.UID),
-							Namespace: appInstance.Status.Namespace,
-							Labels: map[string]string{
-								labels.AcornManaged: "true",
-							},
-						},
-						BinaryData: map[string][]byte{
-							string(appInstance.UID): acornSleepBinary,
-						},
-					})
-					break outer
-				}
-			}
-		}
-	}
-	resp.Objects(deps...)
-	return nil
-}
-
 func toEnvFrom(envs []v1.EnvVar) (result []corev1.EnvFromSource) {
 	for _, env := range envs {
 		if env.Secret.Name != "" && env.Secret.Key == "" {
@@ -765,6 +749,7 @@ func toDeployment(req router.Request, appInstance *v1.AppInstance, tag name.Refe
 }
 
 func ToDeployments(req router.Request, appInstance *v1.AppInstance, tag name.Reference, pullSecrets *PullSecrets, secrets *secrets.Interpolator) (result []kclient.Object, _ error) {
+	var configMapInitialized bool
 	for _, containerName := range typed.SortedKeys(appInstance.Status.AppSpec.Containers) {
 		containerDef := appInstance.Status.AppSpec.Containers[containerName]
 		if ports.IsLinked(appInstance, containerName) {
@@ -779,6 +764,28 @@ func ToDeployments(req router.Request, appInstance *v1.AppInstance, tag name.Ref
 			return nil, err
 		}
 
+		if !configMapInitialized {
+			// This configmap only needs to be created once for the whole app instance.
+			for _, v := range dep.Spec.Template.Spec.Volumes {
+				if v.Name == string(appInstance.UID) {
+					configMapInitialized = true
+					result = append(result, &corev1.ConfigMap{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      string(appInstance.UID),
+							Namespace: appInstance.Status.Namespace,
+							Labels: map[string]string{
+								labels.AcornManaged: "true",
+							},
+						},
+						BinaryData: map[string][]byte{
+							string(appInstance.UID): acornSleepBinary,
+						},
+					})
+					break
+				}
+			}
+		}
+
 		perms := v1.FindPermission(containerName, appInstance.Status.Permissions)
 		sa, err := toServiceAccount(req, dep.GetName(), dep.GetLabels(), dep.GetAnnotations(), appInstance, perms)
 		if err != nil {
@@ -793,5 +800,6 @@ func ToDeployments(req router.Request, appInstance *v1.AppInstance, tag name.Ref
 		}
 		result = append(result, sa, dep, pdb.ToPodDisruptionBudget(dep))
 	}
+
 	return result, nil
 }

pkg/controller/appdefinition/jobs.go

@@ -22,15 +22,6 @@ import (
 	kclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-func addJobs(req router.Request, appInstance *v1.AppInstance, tag name.Reference, pullSecrets *PullSecrets, interpolator *secrets.Interpolator, resp router.Response) error {
-	jobs, err := toJobs(req, appInstance, pullSecrets, tag, interpolator)
-	if err != nil {
-		return err
-	}
-	resp.Objects(jobs...)
-	return nil
-}
-
 func stripPruneAndUpdate(annotations map[string]string) map[string]string {
 	result := map[string]string{}
 	for k, v := range annotations {

pkg/controller/appdefinition/router.go

@@ -6,7 +6,6 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/acorn-io/baaah/pkg/router"
 	"github.com/acorn-io/baaah/pkg/typed"
 	v1 "github.com/acorn-io/runtime/pkg/apis/internal.acorn.io/v1"
 	"github.com/acorn-io/runtime/pkg/labels"
@@ -24,15 +23,6 @@ import (
 	kclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-func addRouters(appInstance *v1.AppInstance, resp router.Response) error {
-	routers, err := toRouters(appInstance)
-	if err != nil {
-		return err
-	}
-	resp.Objects(routers...)
-	return nil
-}
-
 func toRouters(appInstance *v1.AppInstance) (result []kclient.Object, _ error) {
 	for _, entry := range typed.Sorted(appInstance.Status.AppSpec.Routers) {
 		routerObjects, err := toRouter(appInstance, entry.Key, entry.Value)

pkg/controller/appdefinition/testdata/TestComputeMem.golden

@@ -1,4 +1,32 @@
-`apiVersion: internal.acorn.io/v1
+`apiVersion: v1
+data:
+  .dockerconfigjson: eyJhdXRocyI6eyJpbmRleC5kb2NrZXIuaW8iOnsiYXV0aCI6Ik9nPT0ifX19
+kind: Secret
+metadata:
+  creationTimestamp: null
+  labels:
+    acorn.io/managed: "true"
+    acorn.io/pull-secret: "true"
+  name: byname-pull-
+  namespace: app-created-namespace
+type: kubernetes.io/dockerconfigjson
+
+---
+apiVersion: v1
+data:
+  .dockerconfigjson: eyJhdXRocyI6eyJpbmRleC5kb2NrZXIuaW8iOnsiYXV0aCI6Ik9nPT0ifX19
+kind: Secret
+metadata:
+  creationTimestamp: null
+  labels:
+    acorn.io/managed: "true"
+    acorn.io/pull-secret: "true"
+  name: other-pull-
+  namespace: app-created-namespace
+type: kubernetes.io/dockerconfigjson
+
+---
+apiVersion: internal.acorn.io/v1
 kind: ServiceInstance
 metadata:
   annotations:
@@ -137,34 +165,6 @@ status:
       vcs: {}
   summary: {}
 
----
-apiVersion: v1
-data:
-  .dockerconfigjson: eyJhdXRocyI6eyJpbmRleC5kb2NrZXIuaW8iOnsiYXV0aCI6Ik9nPT0ifX19
-kind: Secret
-metadata:
-  creationTimestamp: null
-  labels:
-    acorn.io/managed: "true"
-    acorn.io/pull-secret: "true"
-  name: byname-pull-
-  namespace: app-created-namespace
-type: kubernetes.io/dockerconfigjson
-
----
-apiVersion: v1
-data:
-  .dockerconfigjson: eyJhdXRocyI6eyJpbmRleC5kb2NrZXIuaW8iOnsiYXV0aCI6Ik9nPT0ifX19
-kind: Secret
-metadata:
-  creationTimestamp: null
-  labels:
-    acorn.io/managed: "true"
-    acorn.io/pull-secret: "true"
-  name: other-pull-
-  namespace: app-created-namespace
-type: kubernetes.io/dockerconfigjson
-
 ---
 apiVersion: internal.acorn.io/v1
 kind: AppInstance

pkg/controller/appdefinition/testdata/acorn/basic/expected.golden

@@ -1,4 +1,18 @@
-`apiVersion: internal.acorn.io/v1
+`apiVersion: v1
+data:
+  .dockerconfigjson: eyJhdXRocyI6eyJpbmRleC5kb2NrZXIuaW8iOnsiYXV0aCI6Ik9nPT0ifX19
+kind: Secret
+metadata:
+  creationTimestamp: null
+  labels:
+    acorn.io/managed: "true"
+    acorn.io/pull-secret: "true"
+  name: acorn-name-pull-1234567890ab
+  namespace: app-created-namespace
+type: kubernetes.io/dockerconfigjson
+
+---
+apiVersion: internal.acorn.io/v1
 kind: ServiceInstance
 metadata:
   annotations:
@@ -126,20 +140,6 @@ status:
       vcs: {}
   summary: {}
 
----
-apiVersion: v1
-data:
-  .dockerconfigjson: eyJhdXRocyI6eyJpbmRleC5kb2NrZXIuaW8iOnsiYXV0aCI6Ik9nPT0ifX19
-kind: Secret
-metadata:
-  creationTimestamp: null
-  labels:
-    acorn.io/managed: "true"
-    acorn.io/pull-secret: "true"
-  name: acorn-name-pull-1234567890ab
-  namespace: app-created-namespace
-type: kubernetes.io/dockerconfigjson
-
 ---
 apiVersion: internal.acorn.io/v1
 kind: AppInstance