Properly communicate missing secrets from secret:// syntax to user

Signed-off-by: Darren Shepherd <[email protected]>

Author: ibuildthecloud

pkg/apis/internal.acorn.io/v1/appstatus.go

@@ -30,6 +30,9 @@ func (e *ExpressionError) String() string {
 	if e.DependencyNotFound == nil {
 		return "error [" + e.Error + "] expression [" + e.Expression + "]"
 	}
+	if e.Expression != "" {
+		return fmt.Sprintf("missing %s [%s] from expression [%s]", e.DependencyNotFound.DependencyType, e.DependencyNotFound.Name, e.Expression)
+	}
 	return fmt.Sprintf("missing %s [%s]", e.DependencyNotFound.DependencyType, e.DependencyNotFound.Name)
 }
 

pkg/apis/internal.acorn.io/v1/unmarshal.go

@@ -273,29 +273,20 @@ func (in *VolumeBinding) UnmarshalJSON(data []byte) error {
 
 func impliedSecretsForContainer(app *AppSpec, container Container) {
 	for _, env := range container.Environment {
-		if strings.Contains(env.Secret.Name, ".") {
-			continue
-		}
 		if _, ok := app.Secrets[env.Secret.Name]; env.Secret.Name != "" && !ok {
 			app.Secrets[env.Secret.Name] = Secret{
 				Type: "opaque",
 			}
 		}
 	}
 	for _, dir := range container.Dirs {
-		if strings.Contains(dir.Secret.Name, ".") {
-			continue
-		}
 		if _, ok := app.Secrets[dir.Secret.Name]; dir.Secret.Name != "" && !ok {
 			app.Secrets[dir.Secret.Name] = Secret{
 				Type: "opaque",
 			}
 		}
 	}
 	for _, file := range container.Files {
-		if strings.Contains(file.Secret.Name, ".") {
-			continue
-		}
 		if _, ok := app.Secrets[file.Secret.Name]; file.Secret.Name != "" && !ok {
 			app.Secrets[file.Secret.Name] = Secret{
 				Type: "opaque",

pkg/controller/appdefinition/deploy.go

@@ -10,7 +10,6 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/acorn-io/baaah/pkg/apply"
 	"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"
@@ -551,36 +550,40 @@ func getRevision(req router.Request, namespace, secretName string) (string, erro
 
 func getSecretAnnotations(req router.Request, appInstance *v1.AppInstance, container v1.Container, interpolator *secrets.Interpolator) (map[string]string, error) {
 	var (
-		secrets []string
-		result  = map[string]string{}
+		secretNames = sets.New[string]()
+		result      = map[string]string{}
 	)
 
 	for _, env := range container.Environment {
 		if env.Secret.OnChange == v1.ChangeTypeRedeploy {
-			secrets = append(secrets, env.Secret.Name)
+			secretNames.Insert(env.Secret.Name)
 		}
 	}
 	for _, file := range container.Files {
 		if file.Secret.OnChange == v1.ChangeTypeRedeploy {
-			secrets = append(secrets, file.Secret.Name)
+			secretNames.Insert(file.Secret.Name)
 		}
 	}
 	for _, dir := range container.Dirs {
 		if dir.Secret.OnChange == v1.ChangeTypeRedeploy {
-			secrets = append(secrets, dir.Secret.Name)
+			secretNames.Insert(dir.Secret.Name)
 		}
 	}
 
-	for _, secret := range secrets {
+	for _, secret := range sets.List(secretNames) {
 		if secret == "" {
 			continue
 		}
 		rev, err := getRevision(req, appInstance.Status.Namespace, secret)
 		if apierror.IsNotFound(err) {
-			if !appInstance.GetStopped() {
-				result[apply.AnnotationUpdate] = "false"
-			}
-			result[apply.AnnotationCreate] = "false"
+			interpolator.AddError(&secrets.ErrInterpolation{
+				ExpressionError: v1.ExpressionError{
+					DependencyNotFound: &v1.DependencyNotFound{
+						DependencyType: v1.DependencySecret,
+						Name:           secret,
+					},
+				},
+			})
 		} else if err != nil {
 			return nil, err
 		}
@@ -659,7 +662,7 @@ func toDeployment(req router.Request, appInstance *v1.AppInstance, tag name.Refe
 	if appInstance.Spec.Stop != nil && *appInstance.Spec.Stop {
 		dep.Spec.Replicas = new(int32)
 	} else {
-		interpolator.AddMissingAnnotations(dep.Annotations)
+		interpolator.AddMissingAnnotations(appInstance.GetStopped(), dep.Annotations)
 	}
 
 	return dep, nil

pkg/controller/appdefinition/jobs.go

@@ -156,7 +156,7 @@ func toJob(req router.Request, appInstance *v1.AppInstance, pullSecrets *PullSec
 		},
 	}
 
-	interpolator.AddMissingAnnotations(baseAnnotations)
+	interpolator.AddMissingAnnotations(appInstance.GetStopped(), baseAnnotations)
 
 	if container.Schedule == "" {
 		jobSpec.BackoffLimit = &[]int32{1000}[0]

pkg/controller/appstatus/appstatus.go

@@ -28,11 +28,13 @@ func PrepareStatus(req router.Request, _ router.Response) error {
 		// dependency status will be set correctly.
 		status.Ready = status.Ready && app.Generation == app.Status.ObservedGeneration
 		status.ExpressionErrors = nil
+		status.Dependencies = nil
 		app.Status.AppStatus.Containers[name] = status
 	}
 
 	for name, status := range app.Status.AppStatus.Jobs {
 		status.ExpressionErrors = nil
+		status.Dependencies = nil
 		if app.Generation != app.Status.ObservedGeneration && jobs.ShouldRun(name, app) {
 			// If a job is going to run again, then set its status to not ready so that the controller will run it again and the
 			// dependency status will be set correctly.

pkg/controller/appstatus/containers.go

@@ -38,6 +38,7 @@ func (a *appStatusRenderer) readContainers() error {
 		cs.LinkOverride = ports.LinkService(a.app, containerName)
 		cs.ErrorMessages = append(cs.ErrorMessages, summary.ErrorMessages...)
 		cs.ExpressionErrors = existingStatus[containerName].ExpressionErrors
+		cs.Dependencies = existingStatus[containerName].Dependencies
 		cs.TransitioningMessages = append(cs.TransitioningMessages, summary.TransitioningMessages...)
 		cs.MaxReplicaRestartCount = summary.MaxReplicaRestartCount
 

pkg/controller/appstatus/jobs.go

@@ -31,6 +31,7 @@ func (a *appStatusRenderer) readJobs() error {
 			CreateEventSucceeded: existingStatus[jobName].CreateEventSucceeded,
 			Skipped:              existingStatus[jobName].Skipped,
 			ExpressionErrors:     existingStatus[jobName].ExpressionErrors,
+			Dependencies:         existingStatus[jobName].Dependencies,
 		}
 		summary := summary[jobName]
 

pkg/secrets/interpolation.go

@@ -123,9 +123,11 @@ func (i *Interpolator) Incomplete() bool {
 	return len(i.incomplete) > 0
 }
 
-func (i *Interpolator) AddMissingAnnotations(annotations map[string]string) {
+func (i *Interpolator) AddMissingAnnotations(stopped bool, annotations map[string]string) {
 	if i.Incomplete() {
-		annotations[apply.AnnotationUpdate] = "false"
+		if !stopped {
+			annotations[apply.AnnotationUpdate] = "false"
+		}
 		annotations[apply.AnnotationCreate] = "false"
 	}
 }
@@ -423,6 +425,12 @@ func (i *Interpolator) saveError(err error) {
 	exprError := v1.ExpressionError{
 		Error: err.Error(),
 	}
+	if ee := (*ErrInterpolation)(nil); errors.As(err, &ee) {
+		exprError = ee.ExpressionError
+		if exprError.Error == "" {
+			exprError.Error = exprError.String()
+		}
+	}
 	if i.containerName != "" {
 		i.incomplete[i.containerName] = true
 		c := i.app.Status.AppStatus.Containers[i.containerName]