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
71 changes: 61 additions & 10 deletions controllers/consoleplugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@ package controllers

import (
"context"
"crypto/md5"
"fmt"
"io"
"os"
"reflect"
"sort"
"strings"

argocommon "github.com/argoproj-labs/argocd-operator/common"
argocdutil "github.com/argoproj-labs/argocd-operator/controllers/argoutil"
Expand Down Expand Up @@ -240,18 +243,45 @@ func securityContextForPlugin() *corev1.SecurityContext {
}
}

var httpdConfig = fmt.Sprintf(`LoadModule ssl_module modules/mod_ssl.so
func TLSVersionToPlugin(tlsVersion string) string {
versionMap := map[string]string{
"VersionTLS12": "TLSv1.2",
"VersionTLS13": "TLSv1.3",
"VersionTLS11": "TLSv1.1",
"VersionTLS10": "TLSv1",
}
if v, ok := versionMap[tlsVersion]; ok {
return v
}
return "" // default fallback
}

// buildHttpdConfig generates httpd.conf with dynamic TLS settings
func (r *ReconcileGitopsService) buildHttpdConfig() string {
httpdConfigBase := fmt.Sprintf(`LoadModule ssl_module modules/mod_ssl.so
Listen %d https
ServerRoot "/etc/httpd"

<VirtualHost *:%d>
DocumentRoot /var/www/html/plugin
SSLEngine on
SSLCertificateFile "/etc/httpd-ssl/certs/tls.crt"
SSLCertificateKeyFile "/etc/httpd-ssl/private/tls.key"
</VirtualHost>`, servicePort, servicePort)
SSLCertificateKeyFile "/etc/httpd-ssl/private/tls.key"`, servicePort, servicePort)
// Add SSLProtocol only if explicitly set
if TLSVersionToPlugin(r.TLSMinVersion) != "" {
httpdConfigBase += fmt.Sprintf("\n\tSSLProtocol %s", TLSVersionToPlugin(r.TLSMinVersion))
}
if TLSVersionToPlugin(r.TLSMinVersion) != "TLSv1.3" && strings.Join(r.TLSCiphers, ":") != "" {
httpdConfigBase += fmt.Sprintf("\n\tSSLCipherSuite %s", strings.Join(r.TLSCiphers, ":"))
}
// Close VirtualHost
httpdConfigBase += "\n</VirtualHost>"
return httpdConfigBase
}

// pluginConfigMap creates the ConfigMap with dynamic httpd.conf
func (r *ReconcileGitopsService) pluginConfigMap() *corev1.ConfigMap {
httpdConfig := r.buildHttpdConfig()

func pluginConfigMap() *corev1.ConfigMap {
cm := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: httpdConfigMapName,
Expand Down Expand Up @@ -337,7 +367,7 @@ func sortTolerations(tolerations []corev1.Toleration) []corev1.Toleration {
return sorted
}

func (r *ReconcileGitopsService) reconcileDeployment(cr *pipelinesv1alpha1.GitopsService, request reconcile.Request) (reconcile.Result, error) {
func (r *ReconcileGitopsService) reconcileDeployment(cr *pipelinesv1alpha1.GitopsService, request reconcile.Request, newPluginConfigMap *corev1.ConfigMap) (reconcile.Result, error) {
reqLogger := logs.WithValues("Request.Namespace", request.Namespace, "Request.Name", request.Name)
newPluginDeployment := pluginDeployment(cr.Spec.ImagePullPolicy)

Expand All @@ -359,6 +389,13 @@ func (r *ReconcileGitopsService) reconcileDeployment(cr *pipelinesv1alpha1.Gitop
newPluginDeployment.Spec.Template.Spec.Tolerations = cr.Spec.Tolerations
}

// ADD THIS: Get ConfigMap and add hash to pod template annotations
configMapHash := getConfigMapHash(newPluginConfigMap)
if newPluginDeployment.Spec.Template.ObjectMeta.Annotations == nil {
newPluginDeployment.Spec.Template.ObjectMeta.Annotations = make(map[string]string)
}
newPluginDeployment.Spec.Template.ObjectMeta.Annotations["httpd-cfg-hash"] = configMapHash

// Check if this Deployment already exists
existingPluginDeployment := &appsv1.Deployment{}

Expand All @@ -381,6 +418,7 @@ func (r *ReconcileGitopsService) reconcileDeployment(cr *pipelinesv1alpha1.Gitop
!equality.Semantic.DeepEqual(existingPluginDeployment.Spec.Replicas, newPluginDeployment.Spec.Replicas) ||
!equality.Semantic.DeepEqual(existingPluginDeployment.Spec.Selector, newPluginDeployment.Spec.Selector) ||
!equality.Semantic.DeepEqual(existingSpecTemplate.Labels, newSpecTemplate.Labels) ||
!equality.Semantic.DeepEqual(existingSpecTemplate.ObjectMeta.Annotations["httpd-cfg-hash"], newSpecTemplate.ObjectMeta.Annotations["httpd-cfg-hash"]) ||
!equality.Semantic.DeepEqual(sortContainers(existingSpecTemplate.Spec.Containers), sortContainers(newSpecTemplate.Spec.Containers)) ||
!equality.Semantic.DeepEqual(sortVolumes(existingSpecTemplate.Spec.Volumes), sortVolumes(newSpecTemplate.Spec.Volumes)) ||
!equality.Semantic.DeepEqual(existingSpecTemplate.Spec.RestartPolicy, newSpecTemplate.Spec.RestartPolicy) ||
Expand All @@ -396,6 +434,7 @@ func (r *ReconcileGitopsService) reconcileDeployment(cr *pipelinesv1alpha1.Gitop
existingPluginDeployment.Spec.Replicas = newPluginDeployment.Spec.Replicas
existingPluginDeployment.Spec.Selector = newPluginDeployment.Spec.Selector
existingSpecTemplate.Labels = newSpecTemplate.Labels
existingSpecTemplate.ObjectMeta.Annotations["httpd-cfg-hash"] = newSpecTemplate.ObjectMeta.Annotations["httpd-cfg-hash"]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Prevent nil-map panic when updating annotations.

If the existing Deployment's pod template does not have any annotations, existingSpecTemplate.ObjectMeta.Annotations will be nil. Attempting to write to a nil map will cause a runtime panic that crashes the reconciliation loop.

🛡️ Proposed fix
+			if existingSpecTemplate.ObjectMeta.Annotations == nil {
+				existingSpecTemplate.ObjectMeta.Annotations = make(map[string]string)
+			}
			existingSpecTemplate.ObjectMeta.Annotations["httpd-cfg-hash"] = newSpecTemplate.ObjectMeta.Annotations["httpd-cfg-hash"]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
existingSpecTemplate.ObjectMeta.Annotations["httpd-cfg-hash"] = newSpecTemplate.ObjectMeta.Annotations["httpd-cfg-hash"]
if existingSpecTemplate.ObjectMeta.Annotations == nil {
existingSpecTemplate.ObjectMeta.Annotations = make(map[string]string)
}
existingSpecTemplate.ObjectMeta.Annotations["httpd-cfg-hash"] = newSpecTemplate.ObjectMeta.Annotations["httpd-cfg-hash"]
🤖 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/consoleplugin.go` at line 437, Initialize
existingSpecTemplate.ObjectMeta.Annotations as a writable map when it is nil
before assigning the "httpd-cfg-hash" entry, while preserving the existing
annotation update behavior.

existingSpecTemplate.Spec.SecurityContext = newSpecTemplate.Spec.SecurityContext
existingSpecTemplate.Spec.Containers = newSpecTemplate.Spec.Containers
existingSpecTemplate.Spec.Volumes = newSpecTemplate.Spec.Volumes
Expand Down Expand Up @@ -487,9 +526,18 @@ func (r *ReconcileGitopsService) reconcileConsolePlugin(instance *pipelinesv1alp
return reconcile.Result{}, nil
}

func (r *ReconcileGitopsService) reconcileConfigMap(instance *pipelinesv1alpha1.GitopsService, request reconcile.Request) (reconcile.Result, error) {
// getConfigMapHash returns MD5 hash of ConfigMap data for change detection
func getConfigMapHash(cm *corev1.ConfigMap) string {
hash := md5.New()
for key, value := range cm.Data {
io.WriteString(hash, key)
io.WriteString(hash, value)
}
return fmt.Sprintf("%x", hash.Sum(nil))
}

func (r *ReconcileGitopsService) reconcileConfigMap(instance *pipelinesv1alpha1.GitopsService, request reconcile.Request, newPluginConfigMap *corev1.ConfigMap) (reconcile.Result, error) {
reqLogger := logs.WithValues("Request.Namespace", request.Namespace, "Request.Name", request.Name)
Comment on lines +529 to 540

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Replace MD5 with SHA256 to ensure FIPS compliance and fix non-deterministic hashing.

The crypto/md5 package is not FIPS-compliant and can cause panics or be blocked in FIPS-enabled OpenShift clusters. Additionally, Go map iteration order is random; if cm.Data ever contains more than one key, the hash will change randomly on every reconciliation and cause continuous unwanted Deployment rollouts.

  • controllers/consoleplugin.go#L529-L540: update getConfigMapHash to use sha256.New() and sort the map keys before iterating to guarantee a deterministic hash.
  • controllers/consoleplugin.go#L5-L5: replace the "crypto/md5" import with "crypto/sha256".
🛠️ Proposed fixes

For the import (controllers/consoleplugin.go#L5-L5):

-	"crypto/md5"
+	"crypto/sha256"

For the function (controllers/consoleplugin.go#L529-L540):

-func getConfigMapHash(cm *corev1.ConfigMap) string {
-	hash := md5.New()
-	for key, value := range cm.Data {
-		io.WriteString(hash, key)
-		io.WriteString(hash, value)
-	}
-	return fmt.Sprintf("%x", hash.Sum(nil))
-}
+func getConfigMapHash(cm *corev1.ConfigMap) string {
+	hash := sha256.New()
+	keys := make([]string, 0, len(cm.Data))
+	for k := range cm.Data {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	for _, k := range keys {
+		io.WriteString(hash, k)
+		io.WriteString(hash, cm.Data[k])
+	}
+	return fmt.Sprintf("%x", hash.Sum(nil))
+}
🧰 Tools
🪛 ast-grep (0.44.1)

[warning] 530-530: Detected use of the 'crypto/md5' package (md5.New / md5.Sum). MD5 is a cryptographically broken, collision-prone hash and must not be used for security purposes such as integrity checks, signatures, or password hashing. Use a strong hash from 'crypto/sha256' (sha256.New / sha256.Sum256) or 'crypto/sha512' instead; for passwords use a dedicated KDF such as golang.org/x/crypto/bcrypt or argon2.
Context: md5.New
Note: [CWE-327] Use of a Broken or Risky Cryptographic Algorithm.

(weak-hash-md5-go)

📍 Affects 1 file
  • controllers/consoleplugin.go#L529-L540 (this comment)
  • controllers/consoleplugin.go#L5-L5
🤖 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/consoleplugin.go` around lines 529 - 540, Replace the crypto/md5
import with crypto/sha256 in controllers/consoleplugin.go at lines 5-5. Update
getConfigMapHash at lines 529-540 to create a SHA-256 hash, collect and
lexicographically sort cm.Data keys before writing each key and value, and
preserve the existing hexadecimal string return format.

Source: Linters/SAST tools

Comment on lines +539 to 540

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 r.Client.Update target to avoid missing ResourceVersion error.

Although not directly modified in this PR, the unchanged block at the end of this function incorrectly updates newPluginConfigMap instead of existingPluginConfigMap. Because newPluginConfigMap is freshly generated and lacks a ResourceVersion, Kubernetes will reject the update.

To ensure ConfigMap updates succeed, please fix the target in the existing code block:

// ... in the unchanged code below ...
			existingPluginConfigMap.Data = newPluginConfigMap.Data
			existingPluginConfigMap.Labels = newPluginConfigMap.Labels
-			return reconcile.Result{}, r.Client.Update(context.TODO(), newPluginConfigMap)
+			return reconcile.Result{}, r.Client.Update(context.TODO(), existingPluginConfigMap)
🤖 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/consoleplugin.go` around lines 539 - 540, In
ReconcileGitopsService.reconcileConfigMap, update the existing ConfigMap after
copying Data and Labels by passing existingPluginConfigMap to r.Client.Update
instead of the newly generated newPluginConfigMap, preserving its
ResourceVersion.

newPluginConfigMap := pluginConfigMap()

if err := controllerutil.SetControllerReference(instance, newPluginConfigMap, r.Scheme); err != nil {
return reconcile.Result{}, err
Expand Down Expand Up @@ -529,15 +577,18 @@ func (r *ReconcileGitopsService) reconcilePlugin(instance *pipelinesv1alpha1.Git
return reconcile.Result{}, nil
}

// Generate ConfigMap once
newPluginConfigMap := r.pluginConfigMap()

if result, err := r.reconcileService(instance, request); err != nil {
return result, err
}

if result, err := r.reconcileDeployment(instance, request); err != nil {
if result, err := r.reconcileDeployment(instance, request, newPluginConfigMap); err != nil {
return result, err
}

if result, err := r.reconcileConfigMap(instance, request); err != nil {
if result, err := r.reconcileConfigMap(instance, request, newPluginConfigMap); err != nil {
return result, err
}

Expand Down
Loading
Loading