Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,8 @@ func main() {
Client: client,
Scheme: mgr.GetScheme(),
DisableDefaultInstall: strings.ToLower(os.Getenv(common.DisableDefaultInstallEnvVar)) == "true",
TLSMinVersion: string(profile.MinTLSVersion),
TLSCiphers: profile.Ciphers,
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "GitopsService")
os.Exit(1)
Expand Down
47 changes: 39 additions & 8 deletions controllers/gitopsservice_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ type ReconcileGitopsService struct {

// disableDefaultInstall, if true, will ensure that the default ArgoCD instance is not instantiated in the openshift-gitops namespace.
DisableDefaultInstall bool
// TLSMinVersion is the minimum TLS version to use for the GitOps plugin.
TLSMinVersion string
// TLSCiphers is the list of supported TLS ciphers for the GitOps plugin.
TLSCiphers []string
}

// +kubebuilder:rbac:groups=config.openshift.io,resources=authentications,verbs=get;list;watch
Expand Down Expand Up @@ -658,7 +662,7 @@ func (r *ReconcileGitopsService) reconcileBackend(gitopsserviceNamespacedName ty

// Define a new backend Deployment
{
deploymentObj := newBackendDeployment(gitopsserviceNamespacedName, instance.Spec.ImagePullPolicy)
deploymentObj := newBackendDeployment(gitopsserviceNamespacedName, instance.Spec.ImagePullPolicy, r.TLSMinVersion, r.TLSCiphers)

// Add SeccompProfile based on cluster version
util.AddSeccompProfileForOpenShift(r.Client, &deploymentObj.Spec.Template.Spec)
Expand Down Expand Up @@ -796,11 +800,43 @@ func objectMeta(resourceName string, namespace string, opts ...func(*metav1.Obje
return objectMeta
}

func newBackendDeployment(ns types.NamespacedName, crImagePullPolicy corev1.PullPolicy) *appsv1.Deployment {
func TLSVersionToBackend(tlsVersion string) string {
versionMap := map[string]string{
"VersionTLS12": "1.2",
"VersionTLS13": "1.3",
"VersionTLS11": "1.1",
"VersionTLS10": "1",
}
if v, ok := versionMap[tlsVersion]; ok {
return v
}
return "" // default fallback
}

func newBackendDeployment(ns types.NamespacedName, crImagePullPolicy corev1.PullPolicy, TLSMinVersion string, TLSCiphers []string) *appsv1.Deployment {
image := os.Getenv(backendImageEnvName)
if image == "" {
image = backendImage
}
env := []corev1.EnvVar{
{
Name: insecureEnvVar,
Value: insecureEnvVarValue,
},
}
if TLSVersionToBackend(TLSMinVersion) != "" {
env = append(env, corev1.EnvVar{
Name: "TLS_MIN_VERSION",
Value: TLSVersionToBackend(TLSMinVersion),
})
}

if len(TLSCiphers) > 0 {
env = append(env, corev1.EnvVar{
Name: "TLS_CIPHER_SUITES",
Value: strings.Join(TLSCiphers, ":"),
})
}
podSpec := corev1.PodSpec{
Containers: []corev1.Container{
{
Expand All @@ -814,12 +850,7 @@ func newBackendDeployment(ns types.NamespacedName, crImagePullPolicy corev1.Pull
ContainerPort: port, // should come from flag
},
},
Env: []corev1.EnvVar{
{
Name: insecureEnvVar,
Value: insecureEnvVarValue,
},
},
Env: env,
VolumeMounts: []corev1.VolumeMount{
{
MountPath: "/etc/gitops/ssl",
Expand Down
71 changes: 69 additions & 2 deletions controllers/gitopsservice_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,22 +62,89 @@ func TestImageFromEnvVariable(t *testing.T) {
image := "quay.io/org/test"
t.Setenv(backendImageEnvName, image)

deployment := newBackendDeployment(ns, corev1.PullPolicy(corev1.PullIfNotPresent))
deployment := newBackendDeployment(ns, corev1.PullPolicy(corev1.PullIfNotPresent), "", nil)

got := deployment.Spec.Template.Spec.Containers[0].Image
if got != image {
t.Errorf("Image mismatch: got %s, want %s", got, image)
}
})
t.Run("env variable for image not found", func(t *testing.T) {
deployment := newBackendDeployment(ns, corev1.PullPolicy(corev1.PullIfNotPresent))
deployment := newBackendDeployment(ns, corev1.PullPolicy(corev1.PullIfNotPresent), "", nil)

got := deployment.Spec.Template.Spec.Containers[0].Image
if got != backendImage {
t.Errorf("Image mismatch: got %s, want %s", got, backendImage)
}
})

t.Run("TLS Min Version 1.3 and empty ciphers", func(t *testing.T) {
deployment := newBackendDeployment(ns, corev1.PullPolicy(corev1.PullIfNotPresent), string(configv1.VersionTLS13), nil)
var gotTLSMinVersion string
for _, env := range deployment.Spec.Template.Spec.Containers[0].Env {
if env.Name == "TLS_MIN_VERSION" {
gotTLSMinVersion = env.Value
break
}
}
if gotTLSMinVersion != "1.3" {
t.Errorf("TLS Min Version mismatch: got %s, want %s", gotTLSMinVersion, "1.3")
}
})

t.Run("TLS Min Version 1.2 and empty ciphers", func(t *testing.T) {
deployment := newBackendDeployment(ns, corev1.PullPolicy(corev1.PullIfNotPresent), string(configv1.VersionTLS12), nil)
var gotTLSMinVersion string
for _, env := range deployment.Spec.Template.Spec.Containers[0].Env {
if env.Name == "TLS_MIN_VERSION" {
gotTLSMinVersion = env.Value
break
}
}
if gotTLSMinVersion != "1.2" {
t.Errorf("TLS Min Version mismatch: got %s, want %s", gotTLSMinVersion, "1.2")
}
})

t.Run("TLS Min Version 1.3 and single ciphers", func(t *testing.T) {
deployment := newBackendDeployment(ns, corev1.PullPolicy(corev1.PullIfNotPresent), string(configv1.VersionTLS13), []string{"dummy1"})
var gotTLSMinVersion string
var gotTLSCiphers string
for _, env := range deployment.Spec.Template.Spec.Containers[0].Env {
if env.Name == "TLS_MIN_VERSION" {
gotTLSMinVersion = env.Value
break
}
if env.Name == "TLS_CIPHER_SUITES" {
gotTLSCiphers = env.Value
break
}
}
if gotTLSMinVersion != "1.3" && gotTLSCiphers != "dummy1" {
t.Errorf("TLS Min Version mismatch: got %s, want %s", gotTLSMinVersion, "1.3")
t.Errorf("TLS Cipher mismatch: got %s, want %s", gotTLSCiphers, "dummy1")
}
Comment on lines +113 to +126

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Fix flawed test assertions due to early break and && condition.

The break statement inside the loop causes the iteration to exit immediately after finding the first environment variable (TLS_MIN_VERSION), meaning gotTLSCiphers remains empty. Furthermore, using && in the assertion condition hides this bug because the test falsely passes when gotTLSMinVersion is correct, effectively silencing the failure for the missing cipher verification.

  • controllers/gitopsservice_controller_test.go#L113-L126: Remove the break statements from the loop and separate the assertions to check gotTLSMinVersion and gotTLSCiphers independently against "1.3" and "dummy1".
  • controllers/gitopsservice_controller_test.go#L133-L146: Remove the break statements from the loop and separate the assertions to check gotTLSMinVersion and gotTLSCiphers independently against "1.3" and "dummy1:dummy2".
📍 Affects 1 file
  • controllers/gitopsservice_controller_test.go#L113-L126 (this comment)
  • controllers/gitopsservice_controller_test.go#L133-L146
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@controllers/gitopsservice_controller_test.go` around lines 113 - 126, In
controllers/gitopsservice_controller_test.go sites 113-126 and 133-146, update
both environment-variable loops to remove the early break statements so
TLS_MIN_VERSION and TLS_CIPHER_SUITES are always collected. Separate the
assertions in each test so gotTLSMinVersion is independently checked against
"1.3" and gotTLSCiphers is independently checked against "dummy1" at 113-126 and
"dummy1:dummy2" at 133-146.

})

t.Run("TLS Min Version 1.3 and double ciphers", func(t *testing.T) {
deployment := newBackendDeployment(ns, corev1.PullPolicy(corev1.PullIfNotPresent), string(configv1.VersionTLS13), []string{"dummy1", "dummy2"})
var gotTLSMinVersion string
var gotTLSCiphers string
for _, env := range deployment.Spec.Template.Spec.Containers[0].Env {
if env.Name == "TLS_MIN_VERSION" {
gotTLSMinVersion = env.Value
break
}
if env.Name == "TLS_CIPHER_SUITES" {
gotTLSCiphers = env.Value
break
}
}
if gotTLSMinVersion != "1.3" && gotTLSCiphers != "dummy1:dummy2" {
t.Errorf("TLS Min Version mismatch: got %s, want %s", gotTLSMinVersion, "1.3")
t.Errorf("TLS Cipher mismatch: got %s, want %s", gotTLSCiphers, "dummy1:dummy2")
}
})
}

func TestReconcileDefaultForArgoCDNodeplacement(t *testing.T) {
Expand Down
Loading