Skip to content

chore(tests): Align 1-120_repo_server_system_ca_trust.go with downstream #2158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jannfis merged 1 commit into from
Apr 16, 2026
+20 −25
Merged

chore(tests): Align 1-120_repo_server_system_ca_trust.go with downstream #2158

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 20 additions & 25 deletions tests/ginkgo/sequential/1-120_repo_server_system_ca_trust.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,6 @@ import (
)

var (
// The differences between the upstream image using Ubuntu, and the downstream one using rhel.
image = "" // argocd-operator default
imageVersion = "" // argocd-operator default
caBundlePath = "/etc/ssl/certs/ca-certificates.crt"

trustedHelmAppSource = &appv1alpha1.ApplicationSource{
RepoURL: "https://stefanprodan.github.io/podinfo",
Chart: "podinfo",
Expand All @@ -74,14 +69,17 @@ var (
Helm: &appv1alpha1.ApplicationSourceHelm{Values: "service:\n type: ClusterIP"},
}

k8sClient client.Client
ctx context.Context
k8sClient client.Client
ctx context.Context
ns *corev1.Namespace
cleanupNs func()
image string
imageVersion string

clusterSupportsClusterTrustBundles bool
)

var _ = Describe("GitOps Operator Sequential E2E Tests", func() {

Context("1-120_repo_server_system_ca_trust", func() {
BeforeEach(func() {
fixture.EnsureSequentialCleanSlate()
Expand All @@ -93,12 +91,13 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() {
})

AfterEach(func() {
fixture.OutputDebugOnFail(ns)
cleanupNs()
purgeCtbs()
})

It("ensures that missing Secret aborts startup", func() {
ns, cleanupFunc := fixture.CreateRandomE2ETestNamespaceWithCleanupFunc()
defer cleanupFunc()
ns, cleanupNs = fixture.CreateRandomE2ETestNamespaceWithCleanupFunc()

By("creating Argo CD instance with missing Secret")
argoCD := argoCDSpec(ns, argov1beta1api.ArgoCDRepoSpec{
Expand All @@ -120,8 +119,7 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() {
Skip("Cluster does not support ClusterTrustBundles")
}

ns, cleanupFunc := fixture.CreateRandomE2ETestNamespaceWithCleanupFunc()
defer cleanupFunc()
ns, cleanupNs = fixture.CreateRandomE2ETestNamespaceWithCleanupFunc()

// Create a bundle with 2 CA certs in it. Ubuntu's update-ca-certificates issues a warning, but apparently it works
// It is desirable to test with multiple certs in one bundle because OpenShift permits it
Expand Down Expand Up @@ -161,8 +159,7 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() {
})

It("ensures that CMs and Secrets are trusted in repo-server and plugins", func() {
ns, cleanupFunc := fixture.CreateRandomE2ETestNamespaceWithCleanupFunc()
defer cleanupFunc()
ns, cleanupNs = fixture.CreateRandomE2ETestNamespaceWithCleanupFunc()

cmCert := createCmFromCert(ns, getCACert("github.com"))
Expect(k8sClient.Create(ctx, cmCert)).To(Succeed())
Expand Down Expand Up @@ -210,8 +207,7 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() {
})

It("ensures that 0 trusted certs with DropImageCertificates trusts nothing", func() {
ns, cleanupFunc := fixture.CreateRandomE2ETestNamespaceWithCleanupFunc()
defer cleanupFunc()
ns, cleanupNs = fixture.CreateRandomE2ETestNamespaceWithCleanupFunc()

By("creating Argo CD instance with empty system trust")
argoCD := argoCDSpec(ns, argov1beta1api.ArgoCDRepoSpec{
Expand Down Expand Up @@ -243,8 +239,7 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() {
})

It("ensures that empty trust keeps image certs in place", func() {
ns, cleanupFunc := fixture.CreateRandomE2ETestNamespaceWithCleanupFunc()
defer cleanupFunc()
ns, cleanupNs = fixture.CreateRandomE2ETestNamespaceWithCleanupFunc()

By("creating Argo CD instance with empty system trust")
argoCD := argoCDSpec(ns, argov1beta1api.ArgoCDRepoSpec{
Expand All @@ -258,8 +253,7 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() {
})

It("ensures that Secrets and ConfigMaps get reconciled", func() {
ns, cleanupFunc := fixture.CreateRandomE2ETestNamespaceWithCleanupFunc()
defer cleanupFunc()
ns, cleanupNs = fixture.CreateRandomE2ETestNamespaceWithCleanupFunc()

By("creating Argo CD instance with empty system trust, but full of anticipation")
argoCD := argoCDSpec(ns, argov1beta1api.ArgoCDRepoSpec{
Expand Down Expand Up @@ -350,8 +344,7 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() {
Skip("Cluster does not support ClusterTrustBundles")
}

ns, cleanupFunc := fixture.CreateRandomE2ETestNamespaceWithCleanupFunc()
defer cleanupFunc()
ns, cleanupNs = fixture.CreateRandomE2ETestNamespaceWithCleanupFunc()

combinedCtb := createCtbFromCerts(getCACert("github.com"), getCACert("github.io"))
_ = k8sClient.Delete(ctx, combinedCtb) // Exists only in case of previous failures, must be deleted before argo starts!
Expand Down Expand Up @@ -403,8 +396,7 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() {
Skip("Cluster does not support ClusterTrustBundles")
}

ns, cleanupFunc := fixture.CreateRandomE2ETestNamespaceWithCleanupFunc()
defer cleanupFunc()
ns, cleanupNs = fixture.CreateRandomE2ETestNamespaceWithCleanupFunc()

// Use random label value not to collide with leftover CTBs fom other tests
labelVal := rand.String(5)
Expand Down Expand Up @@ -794,7 +786,9 @@ func getTrustedCertCount(rsPod *corev1.Pod) int {
command := []string{
"kubectl", "-n", rsPod.Namespace, "exec",
"-c", "argocd-repo-server", rsPod.Name, "--",
"cat", caBundlePath,
"bash", "-c",
// Ubuntu or RHEL location
"cat /etc/ssl/certs/ca-certificates.crt || cat /etc/ssl/certs/ca-bundle.crt",
Comment on lines +789 to +791
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 9, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Keep CA bundle path strict to upstream Ubuntu behavior in this test

Using cat ...ca-certificates.crt || ...ca-bundle.crt makes the test accept downstream-specific layout and weakens what this upstream test is verifying.

Suggested fix 
-		"bash", "-c",
-		// Ubuntu or RHEL location
-		"cat /etc/ssl/certs/ca-certificates.crt || cat /etc/ssl/certs/ca-bundle.crt",
+		"cat", "/etc/ssl/certs/ca-certificates.crt",

Based on learnings: In the file tests/ginkgo/sequential/1-120_repo_server_system_ca_trust.go, the CA bundle path should be set to /etc/ssl/certs/ca-certificates.crt on Ubuntu, and downstream RHEL/Fedora/UBI-specific logic should not be mixed in this file.

📝 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
"bash", "-c",
// Ubuntu or RHEL location
"cat /etc/ssl/certs/ca-certificates.crt || cat /etc/ssl/certs/ca-bundle.crt",
"cat", "/etc/ssl/certs/ca-certificates.crt",
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/ginkgo/sequential/1-120_repo_server_system_ca_trust.go` around lines
792 - 794, Replace the permissive shell command string "cat
/etc/ssl/certs/ca-certificates.crt || cat /etc/ssl/certs/ca-bundle.crt" used in
the test with a strict upstream Ubuntu path only: use "cat
/etc/ssl/certs/ca-certificates.crt" so the test asserts the Ubuntu CA bundle
location; update the command literal in the test invocation (the "bash", "-c",
... argument) accordingly and remove downstream RHEL/Fedora/UBI fallback logic.

Comment thread
anandf marked this conversation as resolved.
}

var out string
Expand Down Expand Up @@ -852,6 +846,7 @@ func findRunningRepoServerPod(k8sClient client.Client, ns *corev1.Namespace) *co
pod = runningPods[0]
return nil
}

Copy link
Copy Markdown
Collaborator

@anandf anandf Apr 16, 2026

Choose a reason for hiding this comment

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

nitpick: This empty line can be removed IMO.

return fmt.Errorf("expected exactly one running repo-server pod, found %d", len(runningPods))
}, "40s", "5s").WithOffset(1).Should(Succeed(), "Failed to find Running repo-server pod")

Expand Down