-
Notifications
You must be signed in to change notification settings - Fork 351
fix(repo-server): Fix SystemCATrust for RHEL 9 (WIP) #1123
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,11 +22,14 @@ import ( | |
| "crypto/x509" | ||
| "encoding/pem" | ||
| "fmt" | ||
| "io" | ||
| "net/http" | ||
| "regexp" | ||
| "strings" | ||
|
|
||
| "github.com/onsi/gomega/gcustom" | ||
| matcher "github.com/onsi/gomega/types" | ||
| "gopkg.in/yaml.v3" | ||
| "k8s.io/apimachinery/pkg/util/rand" | ||
| "k8s.io/client-go/util/retry" | ||
| "sigs.k8s.io/controller-runtime/pkg/client/apiutil" | ||
|
|
@@ -56,9 +59,8 @@ 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" | ||
| image = fetchArgoCDComponentImage() | ||
| imageVersion = "main" | ||
|
|
||
| trustedHelmAppSource = &appv1alpha1.ApplicationSource{ | ||
| RepoURL: "https://stefanprodan.github.io/podinfo", | ||
|
|
@@ -76,6 +78,8 @@ var ( | |
|
|
||
| k8sClient client.Client | ||
| ctx context.Context | ||
| ns *corev1.Namespace | ||
| cleanupNs func() | ||
|
|
||
| clusterSupportsClusterTrustBundles bool | ||
| ) | ||
|
|
@@ -90,25 +94,16 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() { | |
| ctx = context.Background() | ||
|
|
||
| clusterSupportsClusterTrustBundles = detectClusterTrustBundleSupport(k8sClient, ctx) | ||
|
|
||
| if fixture.EnvLocalRun() { | ||
| Skip("skipping test as LOCAL_RUN env is set.") | ||
| } | ||
|
|
||
| if !fixture.EnvNonOLM() { | ||
| image = "registry.redhat.io/openshift-gitops-1/argocd-rhel8" | ||
| imageVersion = "sha256:8a0544c14823492165550d83a6d8ba79dd632b46144d3fdcb543793726111d76" | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The very culprit of the bug. The |
||
| caBundlePath = "/etc/ssl/certs/ca-bundle.crt" | ||
| } | ||
| }) | ||
|
|
||
| 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{ | ||
|
|
@@ -130,8 +125,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 | ||
|
|
@@ -171,8 +165,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()) | ||
|
|
@@ -220,8 +213,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{ | ||
|
|
@@ -253,8 +245,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{ | ||
|
|
@@ -268,8 +259,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{ | ||
|
|
@@ -360,8 +350,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! | ||
|
|
@@ -413,8 +402,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) | ||
|
|
@@ -804,7 +792,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", | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done this way, so tests are easier to sync in the future - no difference between operators on this line. |
||
| } | ||
|
|
||
| var out string | ||
|
|
@@ -909,3 +899,35 @@ func purgeCtbs() { | |
| Expect(k8sClient.DeleteAllOf(ctx, &certificatesv1beta1.ClusterTrustBundle{}, expr)).To(Succeed()) | ||
| } | ||
| } | ||
|
|
||
| // fetchArgoCDComponentImage pulls image url to discover its current location | ||
| func fetchArgoCDComponentImage() string { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jgwest, @anandf, @svghadi, I would welcome your opinions here. This pulls the bleeding-edge image location for repo-server ( There are several competing conditions that let me do things this way:
What is ultimately desirable, is to run this with whatever RHEL based argocd images is the operator bundled with*, or the latest ones. *) As I think of this, current impl does not fulfill this criteria. Is there a way to query what image will the operator use to deploy argo components, and inject this only if it is not |
||
| resp, err := http.Get("https://raw.githubusercontent.com/rh-gitops-midstream/release/refs/heads/main/config.yaml") | ||
| Expect(err).ToNot(HaveOccurred(), "failed to fetch config.yaml") | ||
| defer resp.Body.Close() | ||
|
|
||
| Expect(resp.StatusCode).To(Equal(http.StatusOK), "failed to fetch config.yaml") | ||
|
|
||
| body, err := io.ReadAll(resp.Body) | ||
| Expect(err).ToNot(HaveOccurred(), "failed to read config.yaml") | ||
|
|
||
| var config struct { | ||
| KonfluxImages []struct { | ||
| Name string `yaml:"name"` | ||
| BuildRef string `yaml:"buildRef"` | ||
| } `yaml:"konfluxImages"` | ||
| } | ||
|
|
||
| err = yaml.Unmarshal(body, &config) | ||
| Expect(err).ToNot(HaveOccurred(), "failed to parse config.yaml") | ||
|
|
||
| for _, img := range config.KonfluxImages { | ||
| if img.Name == "argocd" { | ||
| Expect(img.BuildRef).ToNot(BeEmpty(), "buildRef for argocd is empty") | ||
| return img.BuildRef | ||
| } | ||
| } | ||
|
|
||
| Fail("argocd image not found in konfluxImages") | ||
| return "" | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the actual fix for the bug. The
--outputoption causesupdate-ca-trustnot to assume it is run as root. Ref.: https://bugzilla.redhat.com/show_bug.cgi?id=2241240