fix env group bugs with excluding default app env (#3601)

Co-authored-by: David Townley <davidtownley@Davids-MacBook-Air.local>

api/server/handlers/environment_groups/create.go

@@ -75,15 +75,10 @@ func (c *UpdateEnvironmentGroupHandler) ServeHTTP(w http.ResponseWriter, r *http
 		return
 	}
 
-	secrets := make(map[string][]byte)
-	for k, v := range request.SecretVariables {
-		secrets[k] = []byte(v)
-	}
-
 	envGroup := environment_groups.EnvironmentGroup{
 		Name:            request.Name,
 		Variables:       request.Variables,
-		SecretVariables: secrets,
+		SecretVariables: request.SecretVariables,
 		CreatedAtUTC:    time.Now().UTC(),
 	}
 

api/server/handlers/environment_groups/list.go

@@ -58,7 +58,7 @@ func (c *ListEnvironmentGroupsHandler) ServeHTTP(w http.ResponseWriter, r *http.
 		return
 	}
 
-	allEnvGroupVersions, err := environmentgroups.ListEnvironmentGroups(ctx, agent, environmentgroups.WithNamespace(environmentgroups.Namespace_EnvironmentGroups))
+	allEnvGroupVersions, err := environmentgroups.ListEnvironmentGroups(ctx, agent, environmentgroups.WithNamespace(environmentgroups.Namespace_EnvironmentGroups), environmentgroups.WithoutDefaultAppEnvironmentGroups())
 	if err != nil {
 		err = telemetry.Error(ctx, span, err, "unable to list all environment groups")
 		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(err, http.StatusInternalServerError))

api/server/handlers/porter_app/get_app_env.go

@@ -131,7 +131,7 @@ func (c *GetAppEnvHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		DeploymentTargetRepository: c.Repo().DeploymentTarget(),
 	}
 
-	envGroups, err := porter_app.AppEnvironmentFromProto(ctx, envFromProtoInp, porter_app.WithEnvGroupFilter(request.EnvGroups), porter_app.WithSecrets())
+	envGroups, err := porter_app.AppEnvironmentFromProto(ctx, envFromProtoInp, porter_app.WithEnvGroupFilter(request.EnvGroups), porter_app.WithSecrets(), porter_app.WithoutDefaultAppEnvGroups())
 	if err != nil {
 		err := telemetry.Error(ctx, span, err, "error getting app environment from revision")
 		c.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(err, http.StatusInternalServerError))

api/server/handlers/porter_app/update_app_environment_group.go

@@ -201,7 +201,7 @@ func (c *UpdateAppEnvironmentHandler) ServeHTTP(w http.ResponseWriter, r *http.R
 	}
 
 	variables := make(map[string]string)
-	secrets := make(map[string][]byte)
+	secrets := make(map[string]string)
 
 	if !request.HardUpdate {
 		for key, value := range latestEnvironmentGroup.Variables {
@@ -216,7 +216,7 @@ func (c *UpdateAppEnvironmentHandler) ServeHTTP(w http.ResponseWriter, r *http.R
 		variables[key] = value
 	}
 	for key, value := range request.Secrets {
-		secrets[key] = []byte(value)
+		secrets[key] = value
 	}
 
 	envGroup := environment_groups.EnvironmentGroup{

internal/kubernetes/environment_groups/create.go

@@ -1,7 +1,6 @@
 package environment_groups
 
 import (
-	"bytes"
 	"context"
 	"fmt"
 	"strconv"
@@ -46,7 +45,7 @@ func CreateOrUpdateBaseEnvironmentGroup(ctx context.Context, a *kubernetes.Agent
 
 	// If any of the secret variables are set to the dummy value (i.e. are unchanged), replace them with the existing value.
 	for k, v := range environmentGroup.SecretVariables {
-		if bytes.Equal(v, []byte(EnvGroupSecretDummyValue)) {
+		if v == EnvGroupSecretDummyValue {
 			existingValue, ok := latestEnvironmentGroup.SecretVariables[k]
 			if !ok {
 				return telemetry.Error(ctx, span, nil, "secret variable does not exist in latest environment group")
@@ -144,6 +143,11 @@ func createVersionedEnvironmentGroupInNamespace(ctx context.Context, a *kubernet
 		return telemetry.Error(ctx, span, err, "unable to create new environment group variables version")
 	}
 
+	secretData := make(map[string][]byte)
+	for k, v := range environmentGroup.SecretVariables {
+		secretData[k] = []byte(v)
+	}
+
 	secret := v1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      fmt.Sprintf("%s.%d", environmentGroup.Name, environmentGroup.Version),
@@ -153,7 +157,10 @@ func createVersionedEnvironmentGroupInNamespace(ctx context.Context, a *kubernet
 				LabelKey_EnvironmentGroupVersion: strconv.Itoa(environmentGroup.Version),
 			},
 		},
-		Data: environmentGroup.SecretVariables,
+		Data: secretData,
+	}
+	for k, v := range additionalLabels {
+		secret.Labels[k] = v
 	}
 
 	err = createSecretWithVersion(ctx, a, secret, environmentGroup.Version)

internal/kubernetes/environment_groups/get.go

@@ -71,9 +71,10 @@ func latestBaseEnvironmentGroup(ctx context.Context, a *kubernetes.Agent, enviro
 // If you are looking for envrionment groups in the base namespace, consider using LatestBaseEnvironmentGroup or ListBaseEnvironmentGroups instead
 type EnvironmentGroupInTargetNamespaceInput struct {
 	// Name is the environment group name which can be found on the configmap label
-	Name      string
-	Version   int
-	Namespace string
+	Name                              string
+	Version                           int
+	Namespace                         string
+	ExcludeDefaultAppEnvironmentGroup bool
 }
 
 // EnvironmentGroupInTargetNamespace checks if an environment group of a specific name and version exists in a target namespace.
@@ -99,7 +100,17 @@ func EnvironmentGroupInTargetNamespace(ctx context.Context, a *kubernetes.Agent,
 		telemetry.AttributeKV{Key: "namespace", Value: inp.Namespace},
 	)
 
-	environmentGroups, err := ListEnvironmentGroups(ctx, a, WithEnvironmentGroupName(inp.Name), WithEnvironmentGroupVersion(inp.Version), WithNamespace(inp.Namespace))
+	opts := []EnvironmentGroupOption{
+		WithEnvironmentGroupName(inp.Name),
+		WithEnvironmentGroupVersion(inp.Version),
+		WithNamespace(inp.Namespace),
+	}
+
+	if inp.ExcludeDefaultAppEnvironmentGroup {
+		opts = append(opts, WithoutDefaultAppEnvironmentGroups())
+	}
+
+	environmentGroups, err := ListEnvironmentGroups(ctx, a, opts...)
 	if err != nil {
 		return eg, telemetry.Error(ctx, span, err, "unable to list environment groups in target namespace")
 	}

internal/kubernetes/environment_groups/list.go

@@ -33,7 +33,7 @@ type EnvironmentGroup struct {
 	// Variables are non-secret values for the EnvironmentGroup. This usually will be a configmap
 	Variables map[string]string `json:"variables,omitempty"`
 	// SecretVariables are secret values for the EnvironmentGroup. This usually will be a Secret on the kubernetes cluster
-	SecretVariables map[string][]byte `json:"secret_variables,omitempty"`
+	SecretVariables map[string]string `json:"secret_variables,omitempty"`
 	// CreatedAt is only used for display purposes and is in UTC Unix time
 	CreatedAtUTC time.Time `json:"created_at"`
 }
@@ -42,7 +42,7 @@ type environmentGroupOptions struct {
 	namespace                          string
 	environmentGroupLabelName          string
 	environmentGroupLabelVersion       int
-	includeDefaultAppEnvironmentGroups bool
+	excludeDefaultAppEnvironmentGroups bool
 }
 
 // EnvironmentGroupOption is a function that modifies ListEnvironmentGroups
@@ -69,10 +69,10 @@ func WithEnvironmentGroupVersion(version int) EnvironmentGroupOption {
 	}
 }
 
-// WithDefaultAppEnvironmentGroup includes default app environment groups in the list
-func WithDefaultAppEnvironmentGroup() EnvironmentGroupOption {
+// WithoutDefaultAppEnvironmentGroups includes default app environment groups in the list
+func WithoutDefaultAppEnvironmentGroups() EnvironmentGroupOption {
 	return func(opts *environmentGroupOptions) {
-		opts.includeDefaultAppEnvironmentGroups = true
+		opts.excludeDefaultAppEnvironmentGroups = true
 	}
 }
 
@@ -133,7 +133,7 @@ func listEnvironmentGroups(ctx context.Context, a *kubernetes.Agent, listOpts ..
 			continue // invalid version label as it should be an int, not an environment group
 		}
 
-		if !opts.includeDefaultAppEnvironmentGroups {
+		if opts.excludeDefaultAppEnvironmentGroups {
 			value := cm.Labels[LabelKey_DefaultAppEnvironment]
 			if value == "true" {
 				continue // do not include default app environment groups
@@ -153,6 +153,11 @@ func listEnvironmentGroups(ctx context.Context, a *kubernetes.Agent, listOpts ..
 	}
 
 	for _, secret := range secretListResp.Items {
+		stringSecret := make(map[string]string)
+		for k, v := range secret.Data {
+			stringSecret[k] = string(v)
+		}
+
 		name, ok := secret.Labels[LabelKey_EnvironmentGroupName]
 		if !ok {
 			continue // missing name label, not an environment group
@@ -166,7 +171,7 @@ func listEnvironmentGroups(ctx context.Context, a *kubernetes.Agent, listOpts ..
 			continue // invalid version label as it should be an int, not an environment group
 		}
 
-		if !opts.includeDefaultAppEnvironmentGroups {
+		if opts.excludeDefaultAppEnvironmentGroups {
 			value, ok := secret.Labels[LabelKey_DefaultAppEnvironment]
 			if ok && value == "true" {
 				continue // do not include default app environment groups
@@ -179,7 +184,7 @@ func listEnvironmentGroups(ctx context.Context, a *kubernetes.Agent, listOpts ..
 		envGroupSet[secret.Name] = EnvironmentGroup{
 			Name:            name,
 			Version:         version,
-			SecretVariables: secret.Data,
+			SecretVariables: stringSecret,
 			Variables:       envGroupSet[secret.Name].Variables,
 			CreatedAtUTC:    secret.CreationTimestamp.Time.UTC(),
 		}
@@ -210,7 +215,7 @@ func ListEnvironmentGroups(ctx context.Context, a *kubernetes.Agent, listOpts ..
 
 	for _, envGroup := range envGroups {
 		for k := range envGroup.SecretVariables {
-			envGroup.SecretVariables[k] = []byte(EnvGroupSecretDummyValue)
+			envGroup.SecretVariables[k] = EnvGroupSecretDummyValue
 		}
 	}
 

internal/porter_app/environment.go

@@ -13,8 +13,9 @@ import (
 )
 
 type envVariarableOptions struct {
-	includeSecrets bool
-	envGroups      []string
+	includeSecrets             bool
+	envGroups                  []string
+	excludeDefaultAppEnvGroups bool
 }
 
 // EnvVariableOption is a function that modifies AppEnvironmentFromProto
@@ -34,6 +35,13 @@ func WithEnvGroupFilter(envGroups []string) EnvVariableOption {
 	}
 }
 
+// WithoutDefaultAppEnvGroups filters out the default app environment groups from the returned list
+func WithoutDefaultAppEnvGroups() EnvVariableOption {
+	return func(opts *envVariarableOptions) {
+		opts.excludeDefaultAppEnvGroups = true
+	}
+}
+
 // AppEnvironmentFromProtoInput is the input struct for AppEnvironmentFromProto
 type AppEnvironmentFromProtoInput struct {
 	ProjectID                  uint
@@ -107,9 +115,10 @@ func AppEnvironmentFromProto(ctx context.Context, inp AppEnvironmentFromProtoInp
 
 	for _, envGroupRef := range filteredEnvGroups {
 		envGroup, err := environment_groups.EnvironmentGroupInTargetNamespace(ctx, inp.K8SAgent, environment_groups.EnvironmentGroupInTargetNamespaceInput{
-			Name:      envGroupRef.GetName(),
-			Version:   int(envGroupRef.GetVersion()),
-			Namespace: namespace,
+			Name:                              envGroupRef.GetName(),
+			Version:                           int(envGroupRef.GetVersion()),
+			Namespace:                         namespace,
+			ExcludeDefaultAppEnvironmentGroup: opts.excludeDefaultAppEnvGroups,
 		})
 		if err != nil {
 			return nil, telemetry.Error(ctx, span, err, "error getting environment group in target namespace")
@@ -119,7 +128,10 @@ func AppEnvironmentFromProto(ctx context.Context, inp AppEnvironmentFromProtoInp
 			envGroup.SecretVariables = nil
 		}
 
-		envGroups = append(envGroups, envGroup)
+		// if envGroup.Name is empty, it means the environment group was a default app environment group and was filtered out
+		if envGroup.Name != "" {
+			envGroups = append(envGroups, envGroup)
+		}
 	}
 
 	return envGroups, nil