(feat) empty proxy env var should result in unset

- If an env variable specified in pod configuration is empty, it will
  override the target PodSpec. For example, if 'foo=""' in pod
  configuration, olm will reset the 'foo' env variable in the target
  PodSpec to 'foo=""'
- However, proxy env var is an exception to the above. An empty proxy
  env var means that it should not be set or it should be unset if it
  is already set in the target PodSpec.

Author: tkashem

pkg/controller/operators/olm/envvar/initializer.go

@@ -56,6 +56,8 @@ func (d *DeploymentInitializer) initialize(ownerCSV ownerutil.Owner, deployment
 			err = fmt.Errorf("failed to query cluster proxy configuration - %v", err)
 			return err
 		}
+
+		proxyEnvVar = dropEmptyProxyEnv(proxyEnvVar)
 	}
 
 	merged = append(podConfigEnvVar, proxyEnvVar...)
@@ -71,3 +73,16 @@ func (d *DeploymentInitializer) initialize(ownerCSV ownerutil.Owner, deployment
 
 	return nil
 }
+
+func dropEmptyProxyEnv(in []corev1.EnvVar) (out []corev1.EnvVar) {
+	out = make([]corev1.EnvVar, 0)
+	for i := range in {
+		if in[i].Value == "" {
+			continue
+		}
+
+		out = append(out, in[i])
+	}
+
+	return
+}

pkg/controller/operators/olm/envvar/inject.go

@@ -29,18 +29,15 @@ func merge(containerEnvVars []corev1.EnvVar, newEnvVars []corev1.EnvVar) (merged
 
 	for _, newEnvVar := range newEnvVars {
 		existing, found := find(containerEnvVars, newEnvVar.Name)
-		if !found {
-			if newEnvVar.Value != "" {
-				merged = append(merged, corev1.EnvVar{
-					Name:  newEnvVar.Name,
-					Value: newEnvVar.Value,
-				})
-			}
-
+		if found {
+			existing.Value = newEnvVar.Value
 			continue
 		}
 
-		existing.Value = newEnvVar.Value
+		merged = append(merged, corev1.EnvVar{
+			Name:  newEnvVar.Name,
+			Value: newEnvVar.Value,
+		})
 	}
 
 	return

test/e2e/subscription_e2e_test.go

@@ -1141,19 +1141,27 @@ func TestCreateNewSubscriptionWithPodConfig(t *testing.T) {
 		}
 
 		require.NotNil(t, proxy)
-		proxyEnv := []corev1.EnvVar{
-			corev1.EnvVar{
+		proxyEnv := []corev1.EnvVar{}
+
+		if proxy.Status.HTTPProxy !="" {
+			proxyEnv = append(proxyEnv, corev1.EnvVar{
 				Name:  "HTTP_PROXY",
 				Value: proxy.Status.HTTPProxy,
-			},
-			corev1.EnvVar{
+			})
+		}
+
+		if proxy.Status.HTTPSProxy !="" {
+			proxyEnv = append(proxyEnv, corev1.EnvVar{
 				Name:  "HTTPS_PROXY",
 				Value: proxy.Status.HTTPSProxy,
-			},
-			corev1.EnvVar{
+			})
+		}
+
+		if proxy.Status.NoProxy !="" {
+			proxyEnv = append(proxyEnv, corev1.EnvVar{
 				Name:  "NO_PROXY",
 				Value: proxy.Status.NoProxy,
-			},
+			})
 		}
 
 		return proxyEnv