Skip to content

Commit

Permalink
fix env group bugs with excluding default app env (#3601)
Browse files Browse the repository at this point in the history
Co-authored-by: David Townley <[email protected]>
  • Loading branch information
d-g-town and David Townley authored Sep 19, 2023
1 parent 8282c88 commit b99a90e
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 32 deletions.
7 changes: 1 addition & 6 deletions api/server/handlers/environment_groups/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}

Expand Down
2 changes: 1 addition & 1 deletion api/server/handlers/environment_groups/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion api/server/handlers/porter_app/get_app_env.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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{
Expand Down
13 changes: 10 additions & 3 deletions internal/kubernetes/environment_groups/create.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package environment_groups

import (
"bytes"
"context"
"fmt"
"strconv"
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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),
Expand All @@ -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)
Expand Down
19 changes: 15 additions & 4 deletions internal/kubernetes/environment_groups/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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")
}
Expand Down
23 changes: 14 additions & 9 deletions internal/kubernetes/environment_groups/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand All @@ -42,7 +42,7 @@ type environmentGroupOptions struct {
namespace string
environmentGroupLabelName string
environmentGroupLabelVersion int
includeDefaultAppEnvironmentGroups bool
excludeDefaultAppEnvironmentGroups bool
}

// EnvironmentGroupOption is a function that modifies ListEnvironmentGroups
Expand All @@ -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
}
}

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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(),
}
Expand Down Expand Up @@ -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
}
}

Expand Down
24 changes: 18 additions & 6 deletions internal/porter_app/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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")
Expand All @@ -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
Expand Down

0 comments on commit b99a90e

Please sign in to comment.