Skip to content

Commit

Permalink
Ensure docker registry CA is trusted in e2e tests
Browse files Browse the repository at this point in the history
  • Loading branch information
thetechnick committed Sep 10, 2024
1 parent 52246e1 commit 33e3fc3
Show file tree
Hide file tree
Showing 9 changed files with 158 additions and 15 deletions.
4 changes: 3 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,12 @@ verify: tidy fmt vet generate ## Verify the current code generation and lint
lint: $(GOLANGCI_LINT) ## Run golangci linter.
$(GOLANGCI_LINT) run $(GOLANGCI_LINT_ARGS)

## image-registry target has to come after run-latest-release,
## because the image-registry depends on the olm-ca issuer.
.PHONY: test-upgrade-e2e
test-upgrade-e2e: export TEST_CLUSTER_CATALOG_NAME := test-catalog
test-upgrade-e2e: export TEST_CLUSTER_CATALOG_IMAGE := docker-registry.catalogd-e2e.svc:5000/test-catalog:e2e
test-upgrade-e2e: kind-cluster cert-manager build-container kind-load image-registry run-latest-release pre-upgrade-setup only-deploy-manifest wait post-upgrade-checks kind-cluster-cleanup ## Run upgrade e2e tests on a local kind cluster
test-upgrade-e2e: kind-cluster cert-manager build-container kind-load run-latest-release image-registry pre-upgrade-setup only-deploy-manifest wait post-upgrade-checks kind-cluster-cleanup ## Run upgrade e2e tests on a local kind cluster

pre-upgrade-setup:
./test/tools/imageregistry/pre-upgrade-setup.sh ${TEST_CLUSTER_CATALOG_IMAGE} ${TEST_CLUSTER_CATALOG_NAME}
Expand Down
59 changes: 58 additions & 1 deletion cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package main

import (
"crypto/tls"
"crypto/x509"
"flag"
"fmt"
"log"
Expand All @@ -26,6 +27,7 @@ import (
"path/filepath"
"time"

"github.com/go-logr/logr"
"github.com/spf13/pflag"
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
Expand Down Expand Up @@ -81,6 +83,7 @@ func main() {
certFile string
keyFile string
webhookPort int
caCertDir string
)
flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
Expand All @@ -97,6 +100,7 @@ func main() {
flag.StringVar(&certFile, "tls-cert", "", "The certificate file used for serving catalog contents over HTTPS. Requires tls-key.")
flag.StringVar(&keyFile, "tls-key", "", "The key file used for serving catalog contents over HTTPS. Requires tls-cert.")
flag.IntVar(&webhookPort, "webhook-server-port", 9443, "The port that the mutating webhook server serves at.")
flag.StringVar(&caCertDir, "ca-certs-dir", "", "The directory of TLS certificate to use for verifying HTTPS connections to the Catalogd and docker-registry web servers.")
opts := zap.Options{
Development: true,
}
Expand Down Expand Up @@ -175,7 +179,13 @@ func main() {
os.Exit(1)
}

unpacker, err := source.NewDefaultUnpacker(systemNamespace, cacheDir)
certPool, err := newCertPool(caCertDir, ctrl.Log.WithName("cert-pool"))
if err != nil {
setupLog.Error(err, "unable to create CA certificate pool")
os.Exit(1)
}

unpacker, err := source.NewDefaultUnpacker(systemNamespace, cacheDir, certPool)
if err != nil {
setupLog.Error(err, "unable to create unpacker")
os.Exit(1)
Expand Down Expand Up @@ -269,3 +279,50 @@ func podNamespace() string {
}
return string(namespace)
}

// Should share code from operator-controller.
// see: https://issues.redhat.com/browse/OPRUN-3535
func newCertPool(caDir string, log logr.Logger) (*x509.CertPool, error) {
caCertPool, err := x509.SystemCertPool()
if err != nil {
return nil, err
}
if caDir == "" {
return caCertPool, nil
}

dirEntries, err := os.ReadDir(caDir)
if err != nil {
return nil, err
}
count := 0

for _, e := range dirEntries {
file := filepath.Join(caDir, e.Name())
// These might be symlinks pointing to directories, so use Stat() to resolve
fi, err := os.Stat(file)
if err != nil {
return nil, err
}
if fi.IsDir() {
log.Info("skip directory", "name", e.Name())
continue
}
log.Info("load certificate", "name", e.Name())
data, err := os.ReadFile(file)
if err != nil {
return nil, fmt.Errorf("error reading cert file %q: %w", file, err)
}

if ok := caCertPool.AppendCertsFromPEM(data); ok {
count++
}
}

// Found no certs!
if count == 0 {
return nil, fmt.Errorf("no certificates found in %q", caDir)
}

return caCertPool, nil
}
73 changes: 73 additions & 0 deletions cmd/manager/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package main

import (
"crypto/rand"
"crypto/rsa"
"crypto/x509"
"crypto/x509/pkix"
"encoding/pem"
"math/big"
"os"
"testing"
"time"

"github.com/go-logr/logr/testr"
"github.com/stretchr/testify/require"
)

func Test_newCertPool(t *testing.T) {
t.Parallel()

// set up our CA certificate
ca := &x509.Certificate{
SerialNumber: big.NewInt(2019),
Subject: pkix.Name{
Organization: []string{"Company, INC."},
Country: []string{"US"},
Province: []string{""},
Locality: []string{"San Francisco"},
StreetAddress: []string{"Golden Gate Bridge"},
PostalCode: []string{"94016"},
},
NotBefore: time.Now(),
NotAfter: time.Now().AddDate(10, 0, 0),
IsCA: true,
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth},
KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
BasicConstraintsValid: true,
}

// create our private and public key
caPrivKey, err := rsa.GenerateKey(rand.Reader, 4096)
require.NoError(t, err)

// create the CA
caBytes, err := x509.CreateCertificate(rand.Reader, ca, ca, &caPrivKey.PublicKey, caPrivKey)
require.NoError(t, err)

// pem encode
err = os.MkdirAll("testdata/newCertPool/subfolder", 0700)
require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, os.RemoveAll("testdata/newCertPool"))
})

caPEM, err := os.Create("testdata/newCertPool/my.pem")
require.NoError(t, err)
err = pem.Encode(caPEM, &pem.Block{
Type: "CERTIFICATE",
Bytes: caBytes,
})
require.NoError(t, err)

_, err = newCertPool("testdata/newCertPool", testr.New(t))
require.NoError(t, err)
}

func Test_newCertPool_empty(t *testing.T) {
err := os.MkdirAll("testdata/newCertPoolEmpty", 0700)
require.NoError(t, err)

_, err = newCertPool("testdata/newCertPoolEmpty", testr.New(t))
require.EqualError(t, err, `no certificates found in "testdata/newCertPoolEmpty"`)
}
1 change: 1 addition & 0 deletions config/base/manager/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ spec:
matchLabels:
control-plane: catalogd-controller-manager
replicas: 1
minReadySeconds: 5
template:
metadata:
annotations:
Expand Down
3 changes: 3 additions & 0 deletions config/components/tls/patches/manager_deployment_certs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,6 @@
- op: add
path: /spec/template/spec/containers/1/args/-
value: "--tls-key=/var/certs/tls.key"
- op: add
path: /spec/template/spec/containers/1/args/-
value: "--ca-certs-dir=/var/certs"
14 changes: 14 additions & 0 deletions internal/source/image_registry_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"archive/tar"
"context"
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"io/fs"
Expand Down Expand Up @@ -31,6 +32,7 @@ import (
type ImageRegistry struct {
BaseCachePath string
AuthNamespace string
CertPool *x509.CertPool
}

const ConfigDirLabel = "operators.operatorframework.io.index.configs.v1"
Expand All @@ -51,6 +53,18 @@ func (i *ImageRegistry) Unpack(ctx context.Context, catalog *catalogdv1alpha1.Cl
}

remoteOpts := []remote.Option{}
tlsTransport := remote.DefaultTransport.(*http.Transport).Clone()
if tlsTransport.TLSClientConfig == nil {
tlsTransport.TLSClientConfig = &tls.Config{
InsecureSkipVerify: false,
MinVersion: tls.VersionTLS12,
} // nolint:gosec
}
if i.CertPool != nil {
tlsTransport.TLSClientConfig.RootCAs = i.CertPool
}
remoteOpts = append(remoteOpts, remote.WithTransport(tlsTransport))

if catalog.Spec.Source.Image.PullSecret != "" {
chainOpts := k8schain.Options{
ImagePullSecrets: []string{catalog.Spec.Source.Image.PullSecret},
Expand Down
4 changes: 3 additions & 1 deletion internal/source/unpacker.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package source

import (
"context"
"crypto/x509"
"fmt"
"io/fs"
"os"
Expand Down Expand Up @@ -110,7 +111,7 @@ const UnpackCacheDir = "unpack"
// source types.
//
// TODO: refactor NewDefaultUnpacker due to growing parameter list
func NewDefaultUnpacker(namespace, cacheDir string) (Unpacker, error) {
func NewDefaultUnpacker(namespace, cacheDir string, certPool *x509.CertPool) (Unpacker, error) {
unpackPath := path.Join(cacheDir, UnpackCacheDir)
if err := os.MkdirAll(unpackPath, 0700); err != nil {
return nil, fmt.Errorf("creating unpack cache directory: %w", err)
Expand All @@ -120,6 +121,7 @@ func NewDefaultUnpacker(namespace, cacheDir string) (Unpacker, error) {
catalogdv1alpha1.SourceTypeImage: &ImageRegistry{
BaseCachePath: unpackPath,
AuthNamespace: namespace,
CertPool: certPool,
},
}), nil
}
3 changes: 1 addition & 2 deletions test/e2e/unpack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ var _ = Describe("ClusterCatalog Unpacking", func() {
Source: catalogd.CatalogSource{
Type: catalogd.SourceTypeImage,
Image: &catalogd.ImageSource{
Ref: catalogImageRef(),
InsecureSkipTLSVerify: true,
Ref: catalogImageRef(),
},
},
},
Expand Down
12 changes: 2 additions & 10 deletions test/tools/imageregistry/imgreg.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,6 @@ metadata:
name: catalogd-e2e
---
apiVersion: cert-manager.io/v1
kind: Issuer
metadata:
name: selfsigned-issuer
namespace: catalogd-e2e
spec:
selfSigned: {}
---
apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
name: catalogd-e2e-registry
Expand All @@ -25,8 +17,8 @@ spec:
algorithm: ECDSA
size: 256
issuerRef:
name: selfsigned-issuer
kind: Issuer
name: olmv1-ca
kind: ClusterIssuer
group: cert-manager.io
---
apiVersion: apps/v1
Expand Down

0 comments on commit 33e3fc3

Please sign in to comment.