feat: Configuring TLS options from Central TLS Profile#1184
feat: Configuring TLS options from Central TLS Profile#1184akhilnittala wants to merge 10 commits into
Conversation
Signed-off-by: akhil nittala <nakhil@redhat.com>
Signed-off-by: akhil nittala <nakhil@redhat.com>
Signed-off-by: akhil nittala <nakhil@redhat.com>
Signed-off-by: akhil nittala <nakhil@redhat.com>
Signed-off-by: akhil nittala <nakhil@redhat.com>
Signed-off-by: akhil nittala <nakhil@redhat.com>
Signed-off-by: akhil nittala <nakhil@redhat.com>
Signed-off-by: akhil nittala <nakhil@redhat.com>
|
@akhilnittala: The label(s) DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThe PR integrates OpenShift cluster TLS security profile support into the gitops-operator. It adds ChangesCluster TLS Profile Integration
Sequence DiagramsequenceDiagram
participant Operator as main
participant EnvFlag as env / CLI flag
participant APIServer as config.openshift.io/apiservers
participant tlspkg as tlspkg helpers
participant Servers as Webhook + Metrics Servers
participant Watcher as SecurityProfileWatcher
participant Controller as ReconcileArgoCD
Operator->>EnvFlag: read DISABLE_CLUSTER_TLS_PROFILE
EnvFlag-->>Operator: disableClusterTLSProfile bool
Operator->>APIServer: fetch TLSProfileSpec
APIServer-->>Operator: MinTLSVersion, Ciphers
Operator->>tlspkg: build TLS option functions
tlspkg-->>Operator: tlsOpts
Operator->>Servers: apply tlsOpts
Operator->>Watcher: start watching apiservers
Watcher-->>Operator: profile changed → cancel ctx → manager exits
Operator->>Controller: pass CentralTLSConfigProfile
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@cmd/main.go`:
- Around line 156-160: The code creates a bootstrapClient using crclient.New()
and assigns any error to err, but immediately calls
tlspkg.FetchAPIServerTLSProfile() with that bootstrapClient on the next line
without checking if client creation failed. Add an error check immediately after
the crclient.New() call to verify err is nil before proceeding to use
bootstrapClient in the FetchAPIServerTLSProfile call. If err is not nil, handle
it appropriately by returning or exiting early to prevent using an invalid
client.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 719b3612-683d-49e6-8b68-a89bdc3cc706
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
bundle/manifests/gitops-operator.clusterserviceversion.yamlcmd/main.goconfig/manager/manager.yamlconfig/rbac/role.yamlcontrollers/gitopsservice_controller.gogo.mod
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
argoproj-labs/argocd-operator(manual)
Signed-off-by: akhil nittala <nakhil@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@controllers/argocd_metrics_controller.go`:
- Around line 400-401: The comparison at line 400 in the
argocd_metrics_controller.go file compares pointer addresses instead of string
values. The condition
`existingServiceMonitor.Spec.Endpoints[0].TLSConfig.ServerName !=
&desiredMetricsServerName` compares pointer addresses, which are different each
reconciliation cycle even when the string content is identical. Fix this by
dereferencing the pointer from
existingServiceMonitor.Spec.Endpoints[0].TLSConfig.ServerName and comparing it
to the string value desiredMetricsServerName instead, so the comparison checks
actual string equality rather than pointer address equality.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: e2f178de-0bcf-4924-afb8-7ee735285c8c
📒 Files selected for processing (3)
bundle/manifests/gitops-operator.clusterserviceversion.yamlcontrollers/argocd_metrics_controller.gotest/openshift/e2e/ginkgo/parallel/1-104_validate_prometheus_alert_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
argoproj-labs/argocd-operator(manual)
🚧 Files skipped from review as they are similar to previous changes (1)
- bundle/manifests/gitops-operator.clusterserviceversion.yaml
Signed-off-by: akhil nittala <nakhil@redhat.com>
|
@akhilnittala: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
What type of PR is this?
/kind enhancement
What does this PR do / why we need it:
This PR introduces configurability for TLS server settings across all Argo CD-related components managed by the operator, aligning with OpenShift Container Platform (OCP) requirements for Post-Quantum Cryptography (PQC) readiness starting from OCP 4.22.
OCP mandates that all layered products must support streamlined TLS configuration. This change ensures that Argo CD components can comply with future cryptographic standards and evolving security policies.
Have you updated the necessary documentation?
Which issue(s) this PR fixes:
Fixes #?
https://redhat.atlassian.net/browse/GITOPS-9073
Test acceptance criteria:
How to test changes / Special notes to the reviewer:
Deploy Redis with tls enabled. checks arguments with underlying apiServer configuration.