Skip to content

Commit 8b35f35

Browse files
anandfanandf
authored
fix(reposerver): added support for allowing customers to set custom GODEBUG settings in repo server deployments (argoproj-labs#2290)
* added support for allowing customers to set custom GODEBUG settings in repo server deployments Signed-off-by: Anand Francis Joseph <anjoseph@redhat.com> * fixed review comments and corrected FIPS env handling Signed-off-by: Anand Francis Joseph <anjoseph@redhat.com> --------- Signed-off-by: Anand Francis Joseph <anjoseph@redhat.com>
1 parent 7d91062 commit 8b35f35

4 files changed

Lines changed: 363 additions & 40 deletions

File tree

‎controllers/argocd/deployment_test.go‎

Lines changed: 225 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,12 @@ const (
3838
testNoProxy = ".example.com"
3939
)
4040

41-
var (
42-
deploymentNames = []string{
43-
"argocd-repo-server",
44-
"argocd-dex-server",
45-
"argocd-redis",
46-
"argocd-server"}
47-
)
41+
var deploymentNames = []string{
42+
"argocd-repo-server",
43+
"argocd-dex-server",
44+
"argocd-redis",
45+
"argocd-server",
46+
}
4847

4948
type MockTrueFipsChecker struct{}
5049

@@ -77,7 +76,6 @@ func TestReconcileArgoCD_reconcileRepoDeployment_replicas(t *testing.T) {
7776

7877
for _, test := range tests {
7978
t.Run(test.name, func(t *testing.T) {
80-
8179
a := makeTestArgoCD(func(a *argoproj.ArgoCD) {
8280
a.Spec.Repo.Replicas = &test.replicas
8381
})
@@ -333,7 +331,6 @@ func TestReconcileArgoCD_reconcile_ServerDeployment_env(t *testing.T) {
333331
assert.Contains(t, deployment.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{Name: "FOO", Value: "BAR"})
334332
assert.Contains(t, deployment.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{Name: "BAR", Value: "FOO"})
335333
})
336-
337334
}
338335

339336
func TestReconcileArgoCD_reconcileRepoDeployment_env(t *testing.T) {
@@ -547,7 +544,6 @@ func TestReconcileArgoCD_reconcileRepoDeployment_mounts(t *testing.T) {
547544
containsDefaultMount := false
548545

549546
for _, volumeMount := range container.VolumeMounts {
550-
551547
if volumeMount.Name == testMount.Name {
552548
containsTestMount = true
553549
} else if volumeMount.MountPath == "/tmp" {
@@ -571,7 +567,6 @@ func TestReconcileArgoCD_reconcileRepoDeployment_mounts(t *testing.T) {
571567

572568
assert.True(t, containsTestMountVolume, "should contain test-mount molume")
573569
assert.False(t, containsDefaultVolume, "should not contain default tmp volume")
574-
575570
})
576571
}
577572

@@ -645,6 +640,7 @@ func TestReconcileArgoCD_reconcileRepoDeployment_missingInitContainers(t *testin
645640
assert.Len(t, deployment.Spec.Template.Spec.InitContainers, 1)
646641
assert.Equal(t, deployment.Spec.Template.Spec.InitContainers[0].Name, "copyutil")
647642
}
643+
648644
func TestReconcileArgoCD_reconcileRepoDeployment_unexpectedInitContainer(t *testing.T) {
649645
logf.SetLogger(ZapLogger(true))
650646
a := makeTestArgoCD()
@@ -724,7 +720,6 @@ func TestReconcileArgoCD_reconcileRepoDeployment_command(t *testing.T) {
724720
// reconcileRepoDeployments creates a Deployment with the proxy settings from the
725721
// environment propagated.
726722
func TestReconcileArgoCD_reconcileDeployments_proxy(t *testing.T) {
727-
728723
t.Setenv("HTTP_PROXY", testHTTPProxy)
729724
t.Setenv("HTTPS_PROXY", testHTTPSProxy)
730725
t.Setenv("no_proxy", testNoProxy)
@@ -900,8 +895,8 @@ func TestReconcileArgoCD_reconcileDeployments_HA_proxy_with_resources(t *testing
900895
assert.Equal(t, deployment.Spec.Template.Spec.Containers[0].Resources, newResources)
901896
assert.Equal(t, deployment.Spec.Template.Spec.InitContainers[0].Resources, newResources)
902897
assert.Equal(t, deployment.Spec.Strategy.RollingUpdate.MaxSurge, &intstr.IntOrString{IntVal: 0})
903-
904898
}
899+
905900
func TestReconcileArgoCD_reconcileRedisHAProxyDeployment_ModifyContainerSpec(t *testing.T) {
906901
logf.SetLogger(ZapLogger(true))
907902

@@ -1217,7 +1212,7 @@ func TestReconcileArgoCD_reconcileDeployment_nodePlacement(t *testing.T) {
12171212
cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs)
12181213
r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset())
12191214

1220-
err := r.reconcileRepoDeployment(a, false) //can use other deployments as well
1215+
err := r.reconcileRepoDeployment(a, false) // can use other deployments as well
12211216
assert.NoError(t, err)
12221217
deployment := &appsv1.Deployment{}
12231218
err = r.Get(context.TODO(), types.NamespacedName{
@@ -1244,6 +1239,7 @@ func deploymentDefaultNodeSelector() map[string]string {
12441239
}
12451240
return nodeSelector
12461241
}
1242+
12471243
func deploymentDefaultTolerations() []corev1.Toleration {
12481244
toleration := []corev1.Toleration{
12491245
{
@@ -2205,7 +2201,6 @@ func operationProcessors(n int32) argoCDOpt {
22052201
}
22062202

22072203
func Test_UpdateNodePlacement(t *testing.T) {
2208-
22092204
deployment := &appsv1.Deployment{
22102205
ObjectMeta: metav1.ObjectMeta{
22112206
Name: "argocd-sample-server",
@@ -2518,13 +2513,16 @@ func serverDefaultVolumeMounts() []corev1.VolumeMount {
25182513
{
25192514
Name: "ssh-known-hosts",
25202515
MountPath: "/app/config/ssh",
2521-
}, {
2516+
},
2517+
{
25222518
Name: "tls-certs",
25232519
MountPath: "/app/config/tls",
2524-
}, {
2520+
},
2521+
{
25252522
Name: "argocd-repo-server-tls",
25262523
MountPath: "/app/config/server/tls",
2527-
}, {
2524+
},
2525+
{
25282526
Name: common.ArgoCDRedisServerTLSSecretName,
25292527
MountPath: "/app/config/server/tls/redis",
25302528
},
@@ -2570,7 +2568,6 @@ func TestReconcileArgoCD_reconcile_RepoServerChanges(t *testing.T) {
25702568

25712569
for _, test := range tests {
25722570
t.Run(test.name, func(t *testing.T) {
2573-
25742571
a := makeTestArgoCD(func(a *argoproj.ArgoCD) {
25752572
a.Spec.Repo.MountSAToken = test.mountSAToken
25762573
a.Spec.Repo.ServiceAccount = test.serviceAccount
@@ -2756,7 +2753,6 @@ func TestReconcileArgoCD_reconcileRepoDeployment_serviceAccount(t *testing.T) {
27562753

27572754
for _, test := range tests {
27582755
t.Run(test.testName, func(t *testing.T) {
2759-
27602756
a := makeTestArgoCD(func(a *argoproj.ArgoCD) {
27612757
a.Spec.Repo.ServiceAccount = test.serviceAccountName
27622758
})
@@ -2935,7 +2931,6 @@ func Test_getRolloutInitContainer(t *testing.T) {
29352931

29362932
assert.Equalf(t, tt.wantImage, containers[0].Image, "Image check")
29372933
assert.Equalf(t, tt.wantEnv, containers[0].Env, "Env check")
2938-
29392934
})
29402935
}
29412936
}
@@ -2968,7 +2963,6 @@ func TestSetReplicasAndEnvVar_WhenServerReplicasIsDefined(t *testing.T) {
29682963
assert.Len(t, deployment.Spec.Template.Spec.Containers[0].Env, 2)
29692964
assert.Contains(t, deployment.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{Name: "ARGOCD_API_SERVER_REPLICAS", Value: "2"})
29702965
})
2971-
29722966
}
29732967

29742968
func TestReconcileArgoCD_reconcileRepoServerWithFipsEnabled(t *testing.T) {
@@ -3044,6 +3038,215 @@ func TestReconcileArgoCD_reconcileRepoServerWithFipsDisabled(t *testing.T) {
30443038
assert.False(t, foundEnv, "environment GODEBUG must NOT be set when FIPS is disabled")
30453039
}
30463040

3041+
func TestReconcileArgoCD_reconcileRepoServerWithFipsEnabledAndCustomGoDebugEnv(t *testing.T) {
3042+
cr := makeTestArgoCD()
3043+
cr.Spec.Repo.Env = []corev1.EnvVar{
3044+
{
3045+
Name: "GODEBUG",
3046+
Value: "fips140=on,tlsmlkem=0",
3047+
},
3048+
}
3049+
resObjs := []client.Object{cr}
3050+
subresObjs := []client.Object{cr}
3051+
runtimeObjs := []runtime.Object{}
3052+
sch := makeTestReconcilerScheme(argoproj.AddToScheme)
3053+
cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs)
3054+
r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset())
3055+
r.FipsConfigChecker = &MockTrueFipsChecker{}
3056+
repoServerRemote := "https://remote.repo-server.instance"
3057+
3058+
cr.Spec.Repo.Remote = &repoServerRemote
3059+
assert.NoError(t, r.reconcileRepoDeployment(cr, false))
3060+
3061+
d := &appsv1.Deployment{}
3062+
3063+
assert.ErrorContains(t, r.Get(context.TODO(), types.NamespacedName{Name: cr.Name + "-repo-server", Namespace: cr.Namespace}, d),
3064+
"deployments.apps \""+cr.Name+"-repo-server\" not found")
3065+
3066+
// once remote is set to nil, reconciliation should trigger deployment resource creation
3067+
cr.Spec.Repo.Remote = nil
3068+
3069+
assert.NoError(t, r.reconcileRepoDeployment(cr, false))
3070+
assert.NoError(t, r.Get(context.TODO(), types.NamespacedName{Name: cr.Name + "-repo-server", Namespace: cr.Namespace}, d))
3071+
foundGoDebug := false
3072+
foundGoLangFips := false
3073+
for _, env := range d.Spec.Template.Spec.Containers[0].Env {
3074+
if env.Name == "GODEBUG" {
3075+
foundGoDebug = true
3076+
assert.Equal(t, "fips140=on,tlsmlkem=0", env.Value, "GODEBUG environment must be set to user provided value when fips is enabled")
3077+
}
3078+
if env.Name == "GOLANG_FIPS" {
3079+
foundGoLangFips = true
3080+
assert.Equal(t, "0", env.Value, "GOLANG_FIPS environment must be set to 0 when fips is enabled")
3081+
}
3082+
}
3083+
assert.True(t, foundGoDebug, "environment GODEBUG must be set to user provided value when FIPS is enabled")
3084+
assert.True(t, foundGoLangFips, "GOLANG_FIPS env var should be set to 0 when fips140=on is present in GODEBUG")
3085+
}
3086+
3087+
func TestReconcileArgoCD_reconcileRepoServerWithFipsEnabledAndCustomGoDebugEnvFipsOff(t *testing.T) {
3088+
cr := makeTestArgoCD()
3089+
cr.Spec.Repo.Env = []corev1.EnvVar{
3090+
{
3091+
Name: "GODEBUG",
3092+
Value: "fips140=off,tlsmlkem=0",
3093+
},
3094+
}
3095+
resObjs := []client.Object{cr}
3096+
subresObjs := []client.Object{cr}
3097+
runtimeObjs := []runtime.Object{}
3098+
sch := makeTestReconcilerScheme(argoproj.AddToScheme)
3099+
cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs)
3100+
r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset())
3101+
r.FipsConfigChecker = &MockTrueFipsChecker{}
3102+
repoServerRemote := "https://remote.repo-server.instance"
3103+
3104+
cr.Spec.Repo.Remote = &repoServerRemote
3105+
assert.NoError(t, r.reconcileRepoDeployment(cr, false))
3106+
3107+
d := &appsv1.Deployment{}
3108+
3109+
assert.ErrorContains(t, r.Get(context.TODO(), types.NamespacedName{Name: cr.Name + "-repo-server", Namespace: cr.Namespace}, d),
3110+
"deployments.apps \""+cr.Name+"-repo-server\" not found")
3111+
3112+
// once remote is set to nil, reconciliation should trigger deployment resource creation
3113+
cr.Spec.Repo.Remote = nil
3114+
3115+
assert.NoError(t, r.reconcileRepoDeployment(cr, false))
3116+
assert.NoError(t, r.Get(context.TODO(), types.NamespacedName{Name: cr.Name + "-repo-server", Namespace: cr.Namespace}, d))
3117+
foundGoDebug := false
3118+
foundGoLangFips := false
3119+
for _, env := range d.Spec.Template.Spec.Containers[0].Env {
3120+
if env.Name == "GODEBUG" {
3121+
foundGoDebug = true
3122+
assert.Equal(t, "fips140=off,tlsmlkem=0", env.Value, "GODEBUG environment must be set to user provided value when fips is enabled")
3123+
}
3124+
if env.Name == "GOLANG_FIPS" {
3125+
foundGoLangFips = true
3126+
}
3127+
}
3128+
assert.True(t, foundGoDebug, "environment GODEBUG must be set to user provided value when FIPS is enabled")
3129+
assert.False(t, foundGoLangFips, "GOLANG_FIPS env var should not be set when fips140=off is present in GODEBUG")
3130+
}
3131+
3132+
func TestReconcileArgoCD_reconcileRepoServerWithFipsEnabledAndCustomGoDebugEnvFipsMissing(t *testing.T) {
3133+
cr := makeTestArgoCD()
3134+
cr.Spec.Repo.Env = []corev1.EnvVar{
3135+
{
3136+
Name: "GODEBUG",
3137+
Value: "tlsmlkem=0",
3138+
},
3139+
}
3140+
resObjs := []client.Object{cr}
3141+
subresObjs := []client.Object{cr}
3142+
runtimeObjs := []runtime.Object{}
3143+
sch := makeTestReconcilerScheme(argoproj.AddToScheme)
3144+
cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs)
3145+
r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset())
3146+
r.FipsConfigChecker = &MockTrueFipsChecker{}
3147+
repoServerRemote := "https://remote.repo-server.instance"
3148+
3149+
cr.Spec.Repo.Remote = &repoServerRemote
3150+
assert.NoError(t, r.reconcileRepoDeployment(cr, false))
3151+
3152+
d := &appsv1.Deployment{}
3153+
3154+
assert.ErrorContains(t, r.Get(context.TODO(), types.NamespacedName{Name: cr.Name + "-repo-server", Namespace: cr.Namespace}, d),
3155+
"deployments.apps \""+cr.Name+"-repo-server\" not found")
3156+
3157+
// once remote is set to nil, reconciliation should trigger deployment resource creation
3158+
cr.Spec.Repo.Remote = nil
3159+
3160+
assert.NoError(t, r.reconcileRepoDeployment(cr, false))
3161+
assert.NoError(t, r.Get(context.TODO(), types.NamespacedName{Name: cr.Name + "-repo-server", Namespace: cr.Namespace}, d))
3162+
foundGoDebug := false
3163+
foundGoLangFips := false
3164+
for _, env := range d.Spec.Template.Spec.Containers[0].Env {
3165+
if env.Name == "GODEBUG" {
3166+
foundGoDebug = true
3167+
assert.Equal(t, "tlsmlkem=0", env.Value, "GODEBUG environment must be set to user provided value when fips is enabled")
3168+
}
3169+
if env.Name == "GOLANG_FIPS" {
3170+
foundGoLangFips = true
3171+
}
3172+
}
3173+
assert.True(t, foundGoDebug, "environment GODEBUG must be set to user provided value when FIPS is enabled")
3174+
assert.False(t, foundGoLangFips, "GOLANG_FIPS env var should not be set when fips140 setting is missing from GODEBUG")
3175+
}
3176+
3177+
func TestReconcileArgoCD_reconcileRepoServerWithFipsEnabledAndCustomGolangFipsEnv(t *testing.T) {
3178+
cr := makeTestArgoCD()
3179+
cr.Spec.Repo.Env = []corev1.EnvVar{
3180+
{
3181+
Name: "GOLANG_FIPS",
3182+
Value: "1",
3183+
},
3184+
}
3185+
resObjs := []client.Object{cr}
3186+
subresObjs := []client.Object{cr}
3187+
runtimeObjs := []runtime.Object{}
3188+
sch := makeTestReconcilerScheme(argoproj.AddToScheme)
3189+
cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs)
3190+
r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset())
3191+
r.FipsConfigChecker = &MockTrueFipsChecker{}
3192+
3193+
assert.NoError(t, r.reconcileRepoDeployment(cr, false))
3194+
3195+
d := &appsv1.Deployment{}
3196+
assert.NoError(t, r.Get(context.TODO(), types.NamespacedName{Name: cr.Name + "-repo-server", Namespace: cr.Namespace}, d))
3197+
3198+
foundGoDebug := false
3199+
foundGoLangFips := false
3200+
for _, env := range d.Spec.Template.Spec.Containers[0].Env {
3201+
if env.Name == "GODEBUG" {
3202+
foundGoDebug = true
3203+
assert.Equal(t, "fips140=on", env.Value, "GODEBUG must be set to fips140=on when fips is enabled")
3204+
}
3205+
if env.Name == "GOLANG_FIPS" {
3206+
foundGoLangFips = true
3207+
assert.Equal(t, "1", env.Value, "user provided GOLANG_FIPS value must be preserved")
3208+
}
3209+
}
3210+
assert.True(t, foundGoDebug, "environment GODEBUG must be set when FIPS is enabled")
3211+
assert.True(t, foundGoLangFips, "GOLANG_FIPS env var must be present")
3212+
}
3213+
3214+
func TestReconcileArgoCD_reconcileRepoServerWithFipsDisabledAndCustomGoDebugEnv(t *testing.T) {
3215+
cr := makeTestArgoCD()
3216+
cr.Spec.Repo.Env = []corev1.EnvVar{
3217+
{
3218+
Name: "GODEBUG",
3219+
Value: "http2debug=1",
3220+
},
3221+
}
3222+
resObjs := []client.Object{cr}
3223+
subresObjs := []client.Object{cr}
3224+
runtimeObjs := []runtime.Object{}
3225+
sch := makeTestReconcilerScheme(argoproj.AddToScheme)
3226+
cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs)
3227+
r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset())
3228+
r.FipsConfigChecker = &MockFalseFipsChecker{}
3229+
3230+
assert.NoError(t, r.reconcileRepoDeployment(cr, false))
3231+
3232+
d := &appsv1.Deployment{}
3233+
assert.NoError(t, r.Get(context.TODO(), types.NamespacedName{Name: cr.Name + "-repo-server", Namespace: cr.Namespace}, d))
3234+
3235+
foundGoDebug := false
3236+
foundGoLangFips := false
3237+
for _, env := range d.Spec.Template.Spec.Containers[0].Env {
3238+
if env.Name == "GODEBUG" {
3239+
foundGoDebug = true
3240+
assert.Equal(t, "http2debug=1", env.Value, "user provided GODEBUG must be preserved when FIPS is disabled")
3241+
}
3242+
if env.Name == "GOLANG_FIPS" {
3243+
foundGoLangFips = true
3244+
}
3245+
}
3246+
assert.True(t, foundGoDebug, "user provided GODEBUG must be preserved when FIPS is disabled")
3247+
assert.False(t, foundGoLangFips, "GOLANG_FIPS env var should not be set when FIPS is disabled")
3248+
}
3249+
30473250
func TestDeploymentWithLongName(t *testing.T) {
30483251
logf.SetLogger(ZapLogger(true))
30493252

0 commit comments

Comments
 (0)