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
6 changes: 3 additions & 3 deletions controllers/argocd/openshift/openshift.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ func ReconcilerHook(cr *argoapp.ArgoCD, v interface{}, hint string) error {
case cr.Name + "-repo-server":

prodImage := o.Spec.Template.Spec.Containers[0].Image
usingReleasedImages := strings.Contains(prodImage, "registry.redhat.io/openshift-gitops-1/argocd-rhel")
if cr.Spec.Repo.SystemCATrust != nil && usingReleasedImages {
if cr.Spec.Repo.SystemCATrust != nil {
updateSystemCATrustBuilding(cr, o, prodImage, logv)
}
}
Expand Down Expand Up @@ -154,7 +153,8 @@ done
echo "User defined trusted CA files:"
ls /etc/pki/ca-trust/source/anchors/

update-ca-trust
# Specifying the explicit location to turn on the container-aware behavior
update-ca-trust extract --output /etc/pki/ca-trust/extracted
Copy link
Copy Markdown
Collaborator Author

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 --output option causes update-ca-trust not to assume it is run as root. Ref.: https://bugzilla.redhat.com/show_bug.cgi?id=2241240


echo "Trusted anchors:"
trust list
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand All @@ -76,6 +78,8 @@ var (

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

clusterSupportsClusterTrustBundles bool
)
Expand All @@ -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"
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The very culprit of the bug. The rhel8 I hard-coded here prevented our quality gates to spot incompatibility with rhel9, so this passed nicely during CI and QE, but failed spectacularly in production using RHEL 9.

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{
Expand All @@ -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
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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!
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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",
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -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 {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The 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 (quay.io/redhat-user-workloads/rh-openshift-gitops-tenant/argocd-rhel9 ATM), so the tests can use it's main tags for this.

There are several competing conditions that let me do things this way:

  • This feature requires RHEL image to function. Tests against the Ubuntu images would force the tests to be skipped / not verify production-like use-case.
  • We want CI/QE to use OS/RPM versions that are as close to production as possible.
  • Hardcoding image url & sha can backfire - again.

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 /.*rhel.*/? thx

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 ""
}
Loading