From 4e172ff3689137445d952632304f962175671480 Mon Sep 17 00:00:00 2001 From: Alexander Chernov Date: Thu, 25 Jun 2026 22:10:17 +0100 Subject: [PATCH 1/3] feat(ca): support a custom CA certificate for GitLab TLS Runners behind a private or self-signed GitLab CA failed to register with "x509: certificate signed by unknown authority" because neither the operator's API client nor the runner pod trusted the CA. Add a spec-level caCertificate field on Runner and MultiRunner that references a PEM CA bundle from either a Secret or a ConfigMap key (key defaults to ca.crt). When set: - the operator's GitLab API client adds the bundle to the system trust pool and uses it for runner create/verify/delete, fixing the registration error; - the CA is mounted read-only into the runner pod and config.toml's tls-ca-file points at it, so the runner's own connection trusts the CA too. The admission webhooks reject a caCertificate that sets both refs or omits the name. CRDs and deepcopy are regenerated; the manager already had configmap read RBAC. Adds unit tests for CA resolution, config rendering, the deployment mount, and the API client's PEM handling. Fixes: #21 Signed-off-by: Alexander Chernov --- README.md | 38 +++++ api/v1beta2/ca_types.go | 54 +++++++ api/v1beta2/multirunner_types.go | 12 ++ api/v1beta2/multirunner_webhook.go | 6 + api/v1beta2/reg.go | 4 + api/v1beta2/runner_types.go | 12 ++ api/v1beta2/runner_webhook.go | 6 + api/v1beta2/zz_generated.deepcopy.go | 55 +++++++ .../gitlab.k8s.alekc.dev_multirunners.yaml | 35 +++++ .../bases/gitlab.k8s.alekc.dev_runners.yaml | 35 +++++ .../gitlab_v1beta2_runner_custom_ca.yaml | 34 +++++ internal/api/gitlab.go | 29 +++- internal/api/gitlab_test.go | 49 ++++++- internal/controller/resolve_ca_test.go | 136 ++++++++++++++++++ internal/controller/shared.go | 58 +++++++- internal/generate/config.go | 8 ++ internal/generate/config_test.go | 70 +++++++++ internal/types/const.go | 10 ++ internal/types/runner_info.go | 2 + internal/validate/validate.go | 49 +++++++ internal/validate/validate_ca_test.go | 98 +++++++++++++ 21 files changed, 793 insertions(+), 7 deletions(-) create mode 100644 api/v1beta2/ca_types.go create mode 100644 config/samples/gitlab_v1beta2_runner_custom_ca.yaml create mode 100644 internal/controller/resolve_ca_test.go create mode 100644 internal/generate/config_test.go create mode 100644 internal/validate/validate_ca_test.go diff --git a/README.md b/README.md index a2f5435..e054dd0 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,7 @@ Top-level `spec` fields (all optional unless noted): | `log_level` | One of `panic`, `fatal`, `error`, `warning`, `info`, `debug`. | | `log_format` | One of `runner`, `text`, `json`. | | `gitlab_instance_url` | GitLab URL. Defaults to `https://gitlab.com/`. | +| `caCertificate` | Optional PEM CA bundle to verify a private or self-signed GitLab endpoint, used for both the operator's API calls and the runner's own connection. References a `secretKeyRef` or `configMapKeyRef` (see below). | | `executor_config` | Kubernetes executor options, see the [keywords reference](https://docs.gitlab.com/runner/executors/kubernetes.html#configuration-settings). | | `environment` | Custom environment variables injected into the build environment. | | `runner_image` | Override the gitlab-runner image. Defaults to a recent `gitlab/gitlab-runner:alpine-vX.Y.Z`. | @@ -90,6 +91,16 @@ exclusive ways to supply the value: | `value` | The literal token, inline. Convenient for testing. | | `secret_key_ref` | Read the token from a Secret in the runner namespace: `name` (required), `key` (optional, defaults to `token`), `optional` (when `true`, a missing secret or key resolves to an empty token instead of failing). | +### `caCertificate` fields + +Set at most one reference. The named object must live in the runner namespace +and hold a PEM CA bundle. + +| Key | Description | +| --- | --- | +| `secretKeyRef` | Read the CA from a Secret: `name` (required), `key` (optional, defaults to `ca.crt`). | +| `configMapKeyRef` | Read the CA from a ConfigMap: `name` (required), `key` (optional, defaults to `ca.crt`). | + ## Examples ### Bring-your-own token @@ -179,6 +190,33 @@ spec: See `config/samples/gitlab_v1beta2_multirunner.yaml` for a `MultiRunner` example that mixes both authentication modes across entries. +### Custom CA for a self-signed GitLab + +Point `caCertificate` at a Secret or ConfigMap holding the PEM CA bundle. The +operator uses it for its own API calls (fixing `x509: certificate signed by +unknown authority` during registration) and mounts it into the runner so its +jobs trust the endpoint too. + +```yaml +apiVersion: gitlab.k8s.alekc.dev/v1beta2 +kind: Runner +metadata: + name: runner-private-ca +spec: + gitlab_instance_url: https://gitlab.internal.example.com/ + caCertificate: + # one of secretKeyRef or configMapKeyRef; key defaults to ca.crt + configMapKeyRef: + name: gitlab-ca + authentication: + access_token: + secret_key_ref: + name: gitlab-access-token + create_options: + runner_type: project_type + project_id: 1234567 +``` + ## RBAC and namespaces The kubernetes executor permission set (pods and pods/exec, pods/attach, diff --git a/api/v1beta2/ca_types.go b/api/v1beta2/ca_types.go new file mode 100644 index 0000000..b990a98 --- /dev/null +++ b/api/v1beta2/ca_types.go @@ -0,0 +1,54 @@ +package v1beta2 + +import "fmt" + +// DefaultCAKey is the key read from the referenced Secret or ConfigMap when +// CAKeyRef.Key is empty. It matches the Kubernetes convention used by TLS +// Secrets and the cluster root-CA ConfigMap. +const DefaultCAKey = "ca.crt" + +// CASource references a PEM-encoded CA bundle used to verify the GitLab +// endpoint, both for the operator's own API calls and for the runner's +// connection. Exactly one of SecretKeyRef or ConfigMapKeyRef may be set. +type CASource struct { + // SecretKeyRef selects a key in a Secret holding the PEM CA bundle. + // +optional + SecretKeyRef *CAKeyRef `json:"secretKeyRef,omitempty"` + + // ConfigMapKeyRef selects a key in a ConfigMap holding the PEM CA bundle. + // +optional + ConfigMapKeyRef *CAKeyRef `json:"configMapKeyRef,omitempty"` +} + +// CAKeyRef points at a single key inside a Secret or ConfigMap. +type CAKeyRef struct { + // Name of the Secret or ConfigMap. + Name string `json:"name"` + + // Key holding the PEM CA bundle. Defaults to "ca.crt" when empty. + // +optional + Key string `json:"key,omitempty"` +} + +// IsSet reports whether the source selects a CA bundle. +func (c *CASource) IsSet() bool { + return c != nil && (c.SecretKeyRef != nil || c.ConfigMapKeyRef != nil) +} + +// Validate enforces that at most one ref is set and that a set ref names a +// source. A nil source is valid (no custom CA). +func (c *CASource) Validate() error { + if c == nil { + return nil + } + if c.SecretKeyRef != nil && c.ConfigMapKeyRef != nil { + return fmt.Errorf("caCertificate: set only one of secretKeyRef or configMapKeyRef") + } + if c.SecretKeyRef != nil && c.SecretKeyRef.Name == "" { + return fmt.Errorf("caCertificate.secretKeyRef.name is required") + } + if c.ConfigMapKeyRef != nil && c.ConfigMapKeyRef.Name == "" { + return fmt.Errorf("caCertificate.configMapKeyRef.name is required") + } + return nil +} diff --git a/api/v1beta2/multirunner_types.go b/api/v1beta2/multirunner_types.go index 23ca8e6..4ff4499 100644 --- a/api/v1beta2/multirunner_types.go +++ b/api/v1beta2/multirunner_types.go @@ -67,6 +67,12 @@ type MultiRunnerSpec struct { // +optional RunnerSecurityContext *corev1.SecurityContext `json:"runner_security_context,omitempty"` + // CACertificate, when set, provides a PEM CA bundle used to verify the + // GitLab endpoint for both the operator's API calls and every runner + // entry's own connection. Reference it from a Secret or a ConfigMap key. + // +optional + CACertificate *CASource `json:"caCertificate,omitempty"` + Entries []MultiRunnerEntry `json:"entries"` } @@ -171,6 +177,11 @@ func (r *MultiRunner) RunnerImage() string { return DefaultRunnerImage } +// CACertificate returns the custom CA source, or nil if none is configured. +func (r *MultiRunner) CACertificate() *CASource { + return r.Spec.CACertificate +} + // SetObservedGeneration records the spec generation the controller acted on. func (r *MultiRunner) SetObservedGeneration(gen int64) { r.Status.ObservedGeneration = gen @@ -214,6 +225,7 @@ func (r *MultiRunner) RegistrationConfig() []GitlabRegInfo { Name: entry.Name, Auth: entry.Authentication, GitlabUrl: r.Spec.GitlabInstanceURL, + CACertificate: r.Spec.CACertificate, RunnerID: r.Status.RunnerIDs[entry.Name], RegistrationHash: r.Status.RegistrationHashes[entry.Name], } diff --git a/api/v1beta2/multirunner_webhook.go b/api/v1beta2/multirunner_webhook.go index 5547c11..3c46488 100644 --- a/api/v1beta2/multirunner_webhook.go +++ b/api/v1beta2/multirunner_webhook.go @@ -62,11 +62,17 @@ func (w *MultiRunnerWebhook) Default(_ context.Context, r *MultiRunner) error { // ValidateCreate validates every entry's auth and entry-name uniqueness. func (w *MultiRunnerWebhook) ValidateCreate(_ context.Context, r *MultiRunner) (admission.Warnings, error) { + if err := r.Spec.CACertificate.Validate(); err != nil { + return nil, err + } return nil, validateEntries(r, w.AllowedBuildNamespaces) } // ValidateUpdate re-runs entry validation against the updated object. func (w *MultiRunnerWebhook) ValidateUpdate(_ context.Context, _, newObj *MultiRunner) (admission.Warnings, error) { + if err := newObj.Spec.CACertificate.Validate(); err != nil { + return nil, err + } return nil, validateEntries(newObj, w.AllowedBuildNamespaces) } diff --git a/api/v1beta2/reg.go b/api/v1beta2/reg.go index 2fb8405..a29aec8 100644 --- a/api/v1beta2/reg.go +++ b/api/v1beta2/reg.go @@ -17,6 +17,10 @@ type GitlabRegInfo struct { // GitlabUrl is the GitLab instance this runner talks to. GitlabUrl string + // CACertificate, when set, is the custom CA bundle source used to verify + // the GitLab endpoint for this unit's API and runner connection. + CACertificate *CASource + // RunnerID is GitLab's numeric id for a managed runner (0 if unmanaged or // not yet created). RunnerID int diff --git a/api/v1beta2/runner_types.go b/api/v1beta2/runner_types.go index 86fc763..68e5bac 100644 --- a/api/v1beta2/runner_types.go +++ b/api/v1beta2/runner_types.go @@ -76,6 +76,12 @@ type RunnerSpec struct { // context. // +optional RunnerSecurityContext *corev1.SecurityContext `json:"runner_security_context,omitempty"` + + // CACertificate, when set, provides a PEM CA bundle used to verify the + // GitLab endpoint for both the operator's API calls and the runner's own + // connection. Reference it from a Secret or a ConfigMap key. + // +optional + CACertificate *CASource `json:"caCertificate,omitempty"` } // DefaultRunnerImage is the gitlab-runner image used when the spec does not @@ -133,12 +139,18 @@ func (r *Runner) RegistrationConfig() []GitlabRegInfo { Name: r.Name, Auth: r.Spec.Authentication, GitlabUrl: r.Spec.GitlabInstanceURL, + CACertificate: r.Spec.CACertificate, RunnerID: r.Status.RunnerID, TokenExpiresAt: r.Status.TokenExpiresAt, RegistrationHash: r.Status.RegistrationHash, }} } +// CACertificate returns the custom CA source, or nil if none is configured. +func (r *Runner) CACertificate() *CASource { + return r.Spec.CACertificate +} + func (r *Runner) StoreRunnerRegistration(info GitlabRegInfo) { r.Status.RunnerID = info.RunnerID r.Status.TokenExpiresAt = info.TokenExpiresAt diff --git a/api/v1beta2/runner_webhook.go b/api/v1beta2/runner_webhook.go index ea39650..25e207c 100644 --- a/api/v1beta2/runner_webhook.go +++ b/api/v1beta2/runner_webhook.go @@ -67,6 +67,9 @@ func (w *RunnerWebhook) ValidateCreate(_ context.Context, r *Runner) (admission. if err := r.Spec.Authentication.Validate(); err != nil { return nil, err } + if err := r.Spec.CACertificate.Validate(); err != nil { + return nil, err + } return nil, validateKubernetesExecutor(&r.Spec.ExecutorConfig, r.Namespace, w.AllowedBuildNamespaces) } @@ -75,6 +78,9 @@ func (w *RunnerWebhook) ValidateUpdate(_ context.Context, _, newObj *Runner) (ad if err := newObj.Spec.Authentication.Validate(); err != nil { return nil, err } + if err := newObj.Spec.CACertificate.Validate(); err != nil { + return nil, err + } return nil, validateKubernetesExecutor(&newObj.Spec.ExecutorConfig, newObj.Namespace, w.AllowedBuildNamespaces) } diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index 0f94823..fc73391 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -26,6 +26,46 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CAKeyRef) DeepCopyInto(out *CAKeyRef) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CAKeyRef. +func (in *CAKeyRef) DeepCopy() *CAKeyRef { + if in == nil { + return nil + } + out := new(CAKeyRef) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CASource) DeepCopyInto(out *CASource) { + *out = *in + if in.SecretKeyRef != nil { + in, out := &in.SecretKeyRef, &out.SecretKeyRef + *out = new(CAKeyRef) + **out = **in + } + if in.ConfigMapKeyRef != nil { + in, out := &in.ConfigMapKeyRef, &out.ConfigMapKeyRef + *out = new(CAKeyRef) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CASource. +func (in *CASource) DeepCopy() *CASource { + if in == nil { + return nil + } + out := new(CASource) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GitlabAuth) DeepCopyInto(out *GitlabAuth) { *out = *in @@ -60,6 +100,11 @@ func (in *GitlabAuth) DeepCopy() *GitlabAuth { func (in *GitlabRegInfo) DeepCopyInto(out *GitlabRegInfo) { *out = *in in.Auth.DeepCopyInto(&out.Auth) + if in.CACertificate != nil { + in, out := &in.CACertificate, &out.CACertificate + *out = new(CASource) + (*in).DeepCopyInto(*out) + } if in.TokenExpiresAt != nil { in, out := &in.TokenExpiresAt, &out.TokenExpiresAt *out = (*in).DeepCopy() @@ -980,6 +1025,11 @@ func (in *MultiRunnerSpec) DeepCopyInto(out *MultiRunnerSpec) { *out = new(v1.SecurityContext) (*in).DeepCopyInto(*out) } + if in.CACertificate != nil { + in, out := &in.CACertificate, &out.CACertificate + *out = new(CASource) + (*in).DeepCopyInto(*out) + } if in.Entries != nil { in, out := &in.Entries, &out.Entries *out = make([]MultiRunnerEntry, len(*in)) @@ -1308,6 +1358,11 @@ func (in *RunnerSpec) DeepCopyInto(out *RunnerSpec) { *out = new(v1.SecurityContext) (*in).DeepCopyInto(*out) } + if in.CACertificate != nil { + in, out := &in.CACertificate, &out.CACertificate + *out = new(CASource) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RunnerSpec. diff --git a/config/crd/bases/gitlab.k8s.alekc.dev_multirunners.yaml b/config/crd/bases/gitlab.k8s.alekc.dev_multirunners.yaml index a06f938..6f3ed1e 100644 --- a/config/crd/bases/gitlab.k8s.alekc.dev_multirunners.yaml +++ b/config/crd/bases/gitlab.k8s.alekc.dev_multirunners.yaml @@ -46,6 +46,41 @@ spec: spec: description: MultiRunnerSpec defines the desired state of MultiRunner properties: + caCertificate: + description: |- + CACertificate, when set, provides a PEM CA bundle used to verify the + GitLab endpoint for both the operator's API calls and every runner + entry's own connection. Reference it from a Secret or a ConfigMap key. + properties: + configMapKeyRef: + description: ConfigMapKeyRef selects a key in a ConfigMap holding + the PEM CA bundle. + properties: + key: + description: Key holding the PEM CA bundle. Defaults to "ca.crt" + when empty. + type: string + name: + description: Name of the Secret or ConfigMap. + type: string + required: + - name + type: object + secretKeyRef: + description: SecretKeyRef selects a key in a Secret holding the + PEM CA bundle. + properties: + key: + description: Key holding the PEM CA bundle. Defaults to "ca.crt" + when empty. + type: string + name: + description: Name of the Secret or ConfigMap. + type: string + required: + - name + type: object + type: object check_interval: minimum: 3 type: integer diff --git a/config/crd/bases/gitlab.k8s.alekc.dev_runners.yaml b/config/crd/bases/gitlab.k8s.alekc.dev_runners.yaml index 5623796..bcc471a 100644 --- a/config/crd/bases/gitlab.k8s.alekc.dev_runners.yaml +++ b/config/crd/bases/gitlab.k8s.alekc.dev_runners.yaml @@ -157,6 +157,41 @@ spec: type: string type: object type: object + caCertificate: + description: |- + CACertificate, when set, provides a PEM CA bundle used to verify the + GitLab endpoint for both the operator's API calls and the runner's own + connection. Reference it from a Secret or a ConfigMap key. + properties: + configMapKeyRef: + description: ConfigMapKeyRef selects a key in a ConfigMap holding + the PEM CA bundle. + properties: + key: + description: Key holding the PEM CA bundle. Defaults to "ca.crt" + when empty. + type: string + name: + description: Name of the Secret or ConfigMap. + type: string + required: + - name + type: object + secretKeyRef: + description: SecretKeyRef selects a key in a Secret holding the + PEM CA bundle. + properties: + key: + description: Key holding the PEM CA bundle. Defaults to "ca.crt" + when empty. + type: string + name: + description: Name of the Secret or ConfigMap. + type: string + required: + - name + type: object + type: object check_interval: minimum: 3 type: integer diff --git a/config/samples/gitlab_v1beta2_runner_custom_ca.yaml b/config/samples/gitlab_v1beta2_runner_custom_ca.yaml new file mode 100644 index 0000000..8617913 --- /dev/null +++ b/config/samples/gitlab_v1beta2_runner_custom_ca.yaml @@ -0,0 +1,34 @@ +# Runner against a GitLab instance with a private / self-signed CA. +# The CA bundle is read from a ConfigMap key (or a Secret key) in the runner +# namespace; the key defaults to ca.crt. The operator trusts the CA for its own +# API calls and mounts it into the runner so its jobs trust the endpoint too. +apiVersion: v1 +kind: ConfigMap +metadata: + name: gitlab-ca +data: + ca.crt: | + -----BEGIN CERTIFICATE----- + REPLACE WITH YOUR PEM CA BUNDLE + -----END CERTIFICATE----- +--- +apiVersion: gitlab.k8s.alekc.dev/v1beta2 +kind: Runner +metadata: + name: runner-private-ca +spec: + gitlab_instance_url: https://gitlab.internal.example.com/ + caCertificate: + configMapKeyRef: + name: gitlab-ca + # key: ca.crt # optional, this is the default + authentication: + access_token: + secret_key_ref: + name: gitlab-access-token + create_options: + runner_type: project_type + project_id: 1234567 + run_untagged: true + tag_list: + - test-gitlab-runner diff --git a/internal/api/gitlab.go b/internal/api/gitlab.go index 8865f06..2dc859c 100644 --- a/internal/api/gitlab.go +++ b/internal/api/gitlab.go @@ -1,7 +1,11 @@ package api import ( + "crypto/tls" + "crypto/x509" + "fmt" "io" + "net/http" gitlab "gitlab.com/gitlab-org/api/client-go" "gitlab.k8s.alekc.dev/api/v1beta2" @@ -138,13 +142,32 @@ func closeBody(resp *gitlab.Response) { } // NewGitlabClient builds a GitLab API client authenticated with the given -// access token. An empty url defaults to the public gitlab.com instance. -func NewGitlabClient(token, url string) (GitlabClient, error) { +// access token. An empty url defaults to the public gitlab.com instance. When +// caPEM is non-empty its certificates are added to the system trust pool and +// used to verify the GitLab endpoint, so a private or self-signed CA is +// trusted; an empty caPEM keeps the default system trust. +func NewGitlabClient(token, url string, caPEM []byte) (GitlabClient, error) { if url == "" { url = "https://gitlab.com/" } + opts := []gitlab.ClientOptionFunc{gitlab.WithBaseURL(url)} + if len(caPEM) > 0 { + pool, err := x509.SystemCertPool() + if err != nil || pool == nil { + pool = x509.NewCertPool() + } + if !pool.AppendCertsFromPEM(caPEM) { + return nil, fmt.Errorf("custom CA bundle contains no valid PEM certificate") + } + httpClient := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{RootCAs: pool, MinVersion: tls.VersionTLS12}, + }, + } + opts = append(opts, gitlab.WithHTTPClient(httpClient)) + } obj := &gitlabApi{} var err error - obj.gitlabApiClient, err = gitlab.NewClient(token, gitlab.WithBaseURL(url)) + obj.gitlabApiClient, err = gitlab.NewClient(token, opts...) return obj, err } diff --git a/internal/api/gitlab_test.go b/internal/api/gitlab_test.go index fe79a3f..8386d4e 100644 --- a/internal/api/gitlab_test.go +++ b/internal/api/gitlab_test.go @@ -1,7 +1,15 @@ package api import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" "testing" + "time" "gitlab.k8s.alekc.dev/api/v1beta2" ) @@ -9,9 +17,48 @@ import ( // TestGitlabApi_CreateRunner is a smoke test: it exercises the client wiring // against the real endpoint with a throwaway token and ignores the result. func TestGitlabApi_CreateRunner(t *testing.T) { - cl, _ := NewGitlabClient("9Bo36Uxwx6ay-cR-bCLh", "") + cl, _ := NewGitlabClient("9Bo36Uxwx6ay-cR-bCLh", "", nil) _, _ = cl.CreateRunner(v1beta2.RunnerCreateOptions{ RunnerType: "instance_type", TagList: []string{"gitlab-testing-operator"}, }) } + +func TestNewGitlabClient_RejectsInvalidCA(t *testing.T) { + if _, err := NewGitlabClient("tok", "https://gitlab.example.com", []byte("not a pem")); err == nil { + t.Fatal("expected an error for a CA bundle with no valid PEM certificate") + } +} + +func TestNewGitlabClient_AcceptsValidCA(t *testing.T) { + cl, err := NewGitlabClient("tok", "https://gitlab.example.com", selfSignedCAPEM(t)) + if err != nil { + t.Fatalf("unexpected error with a valid CA bundle: %v", err) + } + if cl == nil { + t.Fatal("expected a client, got nil") + } +} + +// selfSignedCAPEM builds a throwaway self-signed CA certificate in PEM form. +func selfSignedCAPEM(t *testing.T) []byte { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generate key: %v", err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "test-ca"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + IsCA: true, + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCertSign, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatalf("create cert: %v", err) + } + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) +} diff --git a/internal/controller/resolve_ca_test.go b/internal/controller/resolve_ca_test.go new file mode 100644 index 0000000..fad4a52 --- /dev/null +++ b/internal/controller/resolve_ca_test.go @@ -0,0 +1,136 @@ +package controller + +import ( + "context" + "testing" + + "gitlab.k8s.alekc.dev/api/v1beta2" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +// TestResolveCABundle covers reading the CA PEM from a Secret or a ConfigMap, +// the default and custom key, the empty source, and the error paths. +func TestResolveCABundle(t *testing.T) { + const ns = "default" + const pemBody = "-----BEGIN CERTIFICATE-----\nMIIBfake\n-----END CERTIFICATE-----\n" + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "ca-secret", Namespace: ns}, + Data: map[string][]byte{ + "ca.crt": []byte(pemBody), + "custom": []byte("custom-secret-ca"), + }, + } + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "ca-cm", Namespace: ns}, + Data: map[string]string{ + "ca.crt": pemBody, + "custom": "custom-cm-ca", + }, + } + build := func() client.Client { + return fake.NewClientBuilder().WithObjects(secret, cm).Build() + } + + cases := []struct { + name string + src *v1beta2.CASource + want string + wantErr bool + }{ + {name: "nil source", src: nil, want: ""}, + {name: "empty source", src: &v1beta2.CASource{}, want: ""}, + { + name: "secret default key", + src: &v1beta2.CASource{SecretKeyRef: &v1beta2.CAKeyRef{Name: "ca-secret"}}, + want: pemBody, + }, + { + name: "secret custom key", + src: &v1beta2.CASource{SecretKeyRef: &v1beta2.CAKeyRef{Name: "ca-secret", Key: "custom"}}, + want: "custom-secret-ca", + }, + { + name: "configmap default key", + src: &v1beta2.CASource{ConfigMapKeyRef: &v1beta2.CAKeyRef{Name: "ca-cm"}}, + want: pemBody, + }, + { + name: "configmap custom key", + src: &v1beta2.CASource{ConfigMapKeyRef: &v1beta2.CAKeyRef{Name: "ca-cm", Key: "custom"}}, + want: "custom-cm-ca", + }, + { + name: "secret missing key", + src: &v1beta2.CASource{SecretKeyRef: &v1beta2.CAKeyRef{Name: "ca-secret", Key: "absent"}}, + wantErr: true, + }, + { + name: "configmap missing key", + src: &v1beta2.CASource{ConfigMapKeyRef: &v1beta2.CAKeyRef{Name: "ca-cm", Key: "absent"}}, + wantErr: true, + }, + { + name: "missing secret", + src: &v1beta2.CASource{SecretKeyRef: &v1beta2.CAKeyRef{Name: "absent"}}, + wantErr: true, + }, + { + name: "missing configmap", + src: &v1beta2.CASource{ConfigMapKeyRef: &v1beta2.CAKeyRef{Name: "absent"}}, + wantErr: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := resolveCABundle(context.Background(), build(), ns, tc.src) + if tc.wantErr { + if err == nil { + t.Fatalf("expected error, got nil (value %q)", string(got)) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(got) != tc.want { + t.Fatalf("got %q, want %q", string(got), tc.want) + } + }) + } +} + +// TestCASourceValidate covers the webhook-level validation rules. +func TestCASourceValidate(t *testing.T) { + cases := []struct { + name string + src *v1beta2.CASource + wantErr bool + }{ + {name: "nil", src: nil}, + {name: "empty", src: &v1beta2.CASource{}}, + {name: "secret ok", src: &v1beta2.CASource{SecretKeyRef: &v1beta2.CAKeyRef{Name: "s"}}}, + {name: "configmap ok", src: &v1beta2.CASource{ConfigMapKeyRef: &v1beta2.CAKeyRef{Name: "c"}}}, + {name: "secret no name", src: &v1beta2.CASource{SecretKeyRef: &v1beta2.CAKeyRef{}}, wantErr: true}, + {name: "configmap no name", src: &v1beta2.CASource{ConfigMapKeyRef: &v1beta2.CAKeyRef{}}, wantErr: true}, + { + name: "both set", + src: &v1beta2.CASource{SecretKeyRef: &v1beta2.CAKeyRef{Name: "s"}, ConfigMapKeyRef: &v1beta2.CAKeyRef{Name: "c"}}, + wantErr: true, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := tc.src.Validate() + if tc.wantErr && err == nil { + t.Fatal("expected error, got nil") + } + if !tc.wantErr && err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} diff --git a/internal/controller/shared.go b/internal/controller/shared.go index aa0860d..96ed0d9 100644 --- a/internal/controller/shared.go +++ b/internal/controller/shared.go @@ -93,6 +93,50 @@ func resolveTokenSource(ctx context.Context, cl client.Client, namespace string, return string(value), nil } +// resolveCABundle resolves a CASource to its PEM bytes, read from the named key +// (defaulting to v1beta2.DefaultCAKey) of a Secret or ConfigMap in namespace. A +// nil or empty source returns nil with no error. +func resolveCABundle(ctx context.Context, cl client.Client, namespace string, src *v1beta2.CASource) ([]byte, error) { + if !src.IsSet() { + return nil, nil + } + switch { + case src.SecretKeyRef != nil: + ref := src.SecretKeyRef + var secret corev1.Secret + if err := cl.Get(ctx, client.ObjectKey{Namespace: namespace, Name: ref.Name}, &secret); err != nil { + return nil, fmt.Errorf("cannot read CA secret %q: %w", ref.Name, err) + } + key := ref.Key + if key == "" { + key = v1beta2.DefaultCAKey + } + data, ok := secret.Data[key] + if !ok { + return nil, fmt.Errorf("CA secret %q has no key %q", ref.Name, key) + } + return data, nil + case src.ConfigMapKeyRef != nil: + ref := src.ConfigMapKeyRef + var cm corev1.ConfigMap + if err := cl.Get(ctx, client.ObjectKey{Namespace: namespace, Name: ref.Name}, &cm); err != nil { + return nil, fmt.Errorf("cannot read CA configmap %q: %w", ref.Name, err) + } + key := ref.Key + if key == "" { + key = v1beta2.DefaultCAKey + } + if data, ok := cm.Data[key]; ok { + return []byte(data), nil + } + if data, ok := cm.BinaryData[key]; ok { + return data, nil + } + return nil, fmt.Errorf("CA configmap %q has no key %q", ref.Name, key) + } + return nil, nil +} + // ensureRunners makes sure every runner unit is authenticated and returns the // resolved authentication tokens keyed by entry name (used to render the config // Secret). It also returns a RequeueAfter so managed-runner token expiry is @@ -167,7 +211,11 @@ func ensureManagedRunner(ctx context.Context, cl client.Client, statusW client.S if accessToken == "" { return nil, fmt.Errorf("managed runner %q requires an access_token (value or secret_key_ref)", reg.Name) } - return api.NewGitlabClient(accessToken, reg.GitlabUrl) + caPEM, err := resolveCABundle(ctx, cl, obj.GetNamespace(), reg.CACertificate) + if err != nil { + return nil, err + } + return api.NewGitlabClient(accessToken, reg.GitlabUrl, caPEM) } switch { @@ -312,11 +360,15 @@ func removeManagedRunners(ctx context.Context, cl client.Client, injected api.Gi // token-based delete is rejected. When neither path succeeds the runner may be // left orphaned and an error is returned so the finalizer can retry. func deleteManagedRunner(ctx context.Context, cl client.Client, injected api.GitlabClient, namespace string, reg v1beta2.GitlabRegInfo, token string, logger logr.Logger) error { + caPEM, err := resolveCABundle(ctx, cl, namespace, reg.CACertificate) + if err != nil { + return err + } // Preferred path: delete by the runner's own token (no access-token scope). if token != "" { gc := injected if gc == nil { - c, err := api.NewGitlabClient("", reg.GitlabUrl) + c, err := api.NewGitlabClient("", reg.GitlabUrl, caPEM) if err != nil { return err } @@ -341,7 +393,7 @@ func deleteManagedRunner(ctx context.Context, cl client.Client, injected api.Git if accessToken == "" { return fmt.Errorf("runner %q: no usable runner token and no access_token for the delete-by-id fallback; it may be orphaned on gitlab", reg.Name) } - c, err := api.NewGitlabClient(accessToken, reg.GitlabUrl) + c, err := api.NewGitlabClient(accessToken, reg.GitlabUrl, caPEM) if err != nil { return err } diff --git a/internal/generate/config.go b/internal/generate/config.go index 279ca05..1930612 100644 --- a/internal/generate/config.go +++ b/internal/generate/config.go @@ -44,6 +44,10 @@ func SingleRunnerConfig(r *v1beta2.Runner, tokens map[string]string) (gitlabConf // resolve the executor namespace via the shared defaulting rule so it // matches the namespace RBAC was provisioned for (crud.BuildNamespaces) runnerConfig.RunnerSettings.Kubernetes.Namespace = r.Spec.ExecutorConfig.EffectiveNamespace(r.Namespace) + // point the runner at the mounted custom CA when one is configured + if r.Spec.CACertificate.IsSet() { + runnerConfig.RunnerCredentials.TLSCAFile = types.CACertFile + } rootConfig := &config.Config{ ListenAddress: ":9090", Concurrent: int(math.Max(1, float64(r.Spec.Concurrent))), @@ -92,6 +96,10 @@ func MultiRunnerConfig(runnerObject *v1beta2.MultiRunner, tokens map[string]stri // resolve the executor namespace via the shared defaulting rule so it // matches the namespace RBAC was provisioned for (crud.BuildNamespaces) runnerConfig.RunnerSettings.Kubernetes.Namespace = executorConfig.EffectiveNamespace(runnerObject.Namespace) + // point each runner at the mounted custom CA when one is configured + if runnerObject.Spec.CACertificate.IsSet() { + runnerConfig.RunnerCredentials.TLSCAFile = types.CACertFile + } runners = append(runners, &runnerConfig) } diff --git a/internal/generate/config_test.go b/internal/generate/config_test.go new file mode 100644 index 0000000..22aca05 --- /dev/null +++ b/internal/generate/config_test.go @@ -0,0 +1,70 @@ +package generate + +import ( + "strings" + "testing" + + "gitlab.k8s.alekc.dev/api/v1beta2" + "gitlab.k8s.alekc.dev/internal/types" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func wantTLSCALine() string { + return `tls-ca-file = "` + types.CACertFile + `"` +} + +func TestSingleRunnerConfig_TLSCAFile(t *testing.T) { + base := &v1beta2.Runner{ + ObjectMeta: metav1.ObjectMeta{Name: "r1", Namespace: "ns"}, + Spec: v1beta2.RunnerSpec{GitlabInstanceURL: "https://gitlab.example.com"}, + } + tokens := map[string]string{"r1": "glrt-token"} + + cfg, _, err := SingleRunnerConfig(base, tokens) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if strings.Contains(cfg, "tls-ca-file") { + t.Fatalf("did not expect tls-ca-file without a CA, got:\n%s", cfg) + } + + withCA := base.DeepCopy() + withCA.Spec.CACertificate = &v1beta2.CASource{ConfigMapKeyRef: &v1beta2.CAKeyRef{Name: "ca-cm"}} + cfg, _, err = SingleRunnerConfig(withCA, tokens) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(cfg, wantTLSCALine()) { + t.Fatalf("expected %q in config, got:\n%s", wantTLSCALine(), cfg) + } +} + +func TestMultiRunnerConfig_TLSCAFile(t *testing.T) { + base := &v1beta2.MultiRunner{ + ObjectMeta: metav1.ObjectMeta{Name: "m1", Namespace: "ns"}, + Spec: v1beta2.MultiRunnerSpec{ + GitlabInstanceURL: "https://gitlab.example.com", + Entries: []v1beta2.MultiRunnerEntry{{Name: "e1"}, {Name: "e2"}}, + }, + } + tokens := map[string]string{"e1": "glrt-a", "e2": "glrt-b"} + + cfg, _, err := MultiRunnerConfig(base, tokens) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if strings.Contains(cfg, "tls-ca-file") { + t.Fatalf("did not expect tls-ca-file without a CA, got:\n%s", cfg) + } + + withCA := base.DeepCopy() + withCA.Spec.CACertificate = &v1beta2.CASource{SecretKeyRef: &v1beta2.CAKeyRef{Name: "ca-secret"}} + cfg, _, err = MultiRunnerConfig(withCA, tokens) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // every entry must carry the CA file + if got := strings.Count(cfg, wantTLSCALine()); got != 2 { + t.Fatalf("expected tls-ca-file on both entries (2), got %d in:\n%s", got, cfg) + } +} diff --git a/internal/types/const.go b/internal/types/const.go index bfec888..11dad47 100644 --- a/internal/types/const.go +++ b/internal/types/const.go @@ -8,3 +8,13 @@ const ConfigMapKeyName = "config.toml" // a managed runner's token from "" so the token never has to // live in the CR status. const ConfigTokenKeyPrefix = "authentication-token-" + +// CACertMountDir is where a custom CA bundle is mounted into the runner pod and +// CACertFileName is the file it is projected to. CACertFile is the absolute +// path written into config.toml as tls-ca-file. The directory sits outside the +// config-Secret mount (/etc/gitlab-runner) to avoid nested mounts. +const ( + CACertMountDir = "/etc/gitlab-certs" + CACertFileName = "ca.crt" + CACertFile = CACertMountDir + "/" + CACertFileName +) diff --git a/internal/types/runner_info.go b/internal/types/runner_info.go index d81ba44..c6f80ee 100644 --- a/internal/types/runner_info.go +++ b/internal/types/runner_info.go @@ -31,6 +31,8 @@ type RunnerInfo interface { SetStatusReady(ready bool) ConfigMapVersion() string RunnerImage() string + // CACertificate returns the custom CA source, or nil when none is set. + CACertificate() *v1beta2.CASource RunnerResources() corev1.ResourceRequirements RunnerImagePullPolicy() corev1.PullPolicy RunnerSecurityContext() *corev1.SecurityContext diff --git a/internal/validate/validate.go b/internal/validate/validate.go index b8fa0af..c516810 100644 --- a/internal/validate/validate.go +++ b/internal/validate/validate.go @@ -5,6 +5,7 @@ import ( "reflect" "github.com/go-logr/logr" + "gitlab.k8s.alekc.dev/api/v1beta2" "gitlab.k8s.alekc.dev/internal/result" "gitlab.k8s.alekc.dev/internal/types" appsv1 "k8s.io/api/apps/v1" @@ -84,6 +85,13 @@ func Deployment(ctx context.Context, cl client.Client, runnerObj types.RunnerInf }, } + // Mount a custom CA bundle into the runner container when configured so the + // runner trusts a private or self-signed GitLab endpoint (config.toml points + // at it via tls-ca-file). + if src := runnerObj.CACertificate(); src.IsSet() { + addCustomCAVolume(&wantedDeployment, src) + } + var existingDeployment appsv1.Deployment err := cl.Get(ctx, client.ObjectKey{ Namespace: runnerObj.GetNamespace(), @@ -130,6 +138,47 @@ func Deployment(ctx context.Context, cl client.Client, runnerObj types.RunnerInf return nil, nil } +// addCustomCAVolume mounts the referenced CA bundle (from a Secret or a +// ConfigMap key) into the runner container at types.CACertFile, read-only, so +// config.toml's tls-ca-file can point at it. The chosen key is projected to a +// fixed filename so the mount path is stable regardless of the source key. +func addCustomCAVolume(dep *appsv1.Deployment, src *v1beta2.CASource) { + const volName = "custom-ca" + var vs corev1.VolumeSource + switch { + case src.SecretKeyRef != nil: + key := src.SecretKeyRef.Key + if key == "" { + key = v1beta2.DefaultCAKey + } + vs.Secret = &corev1.SecretVolumeSource{ + SecretName: src.SecretKeyRef.Name, + Items: []corev1.KeyToPath{{Key: key, Path: types.CACertFileName}}, + } + case src.ConfigMapKeyRef != nil: + key := src.ConfigMapKeyRef.Key + if key == "" { + key = v1beta2.DefaultCAKey + } + vs.ConfigMap = &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{Name: src.ConfigMapKeyRef.Name}, + Items: []corev1.KeyToPath{{Key: key, Path: types.CACertFileName}}, + } + default: + return + } + dep.Spec.Template.Spec.Volumes = append(dep.Spec.Template.Spec.Volumes, corev1.Volume{ + Name: volName, + VolumeSource: vs, + }) + c := &dep.Spec.Template.Spec.Containers[0] + c.VolumeMounts = append(c.VolumeMounts, corev1.VolumeMount{ + Name: volName, + MountPath: types.CACertMountDir, + ReadOnly: true, + }) +} + // Secret renders the runner config.toml into an Opaque Secret named after the // runner, keeping each entry's authentication token under a dedicated // ConfigTokenKeyPrefix key so the controller can recover it on later diff --git a/internal/validate/validate_ca_test.go b/internal/validate/validate_ca_test.go new file mode 100644 index 0000000..a6360e6 --- /dev/null +++ b/internal/validate/validate_ca_test.go @@ -0,0 +1,98 @@ +package validate + +import ( + "context" + "testing" + + "github.com/go-logr/logr" + "gitlab.k8s.alekc.dev/api/v1beta2" + "gitlab.k8s.alekc.dev/internal/types" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestDeployment_CustomCAMount(t *testing.T) { + scheme := runtime.NewScheme() + if err := clientgoscheme.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + if err := v1beta2.AddToScheme(scheme); err != nil { + t.Fatal(err) + } + + newRunner := func(ca *v1beta2.CASource) *v1beta2.Runner { + return &v1beta2.Runner{ + TypeMeta: metav1.TypeMeta{APIVersion: "gitlab.k8s.alekc.dev/v1beta2", Kind: "Runner"}, + ObjectMeta: metav1.ObjectMeta{Name: "r1", Namespace: "ns", UID: "uid-1"}, + Spec: v1beta2.RunnerSpec{CACertificate: ca}, + } + } + + getDeployment := func(t *testing.T, runner *v1beta2.Runner) appsv1.Deployment { + t.Helper() + cl := fake.NewClientBuilder().WithScheme(scheme).Build() + if _, err := Deployment(context.Background(), cl, runner, logr.Discard()); err != nil { + t.Fatalf("Deployment: %v", err) + } + var dep appsv1.Deployment + key := client.ObjectKey{Namespace: "ns", Name: runner.ChildName()} + if err := cl.Get(context.Background(), key, &dep); err != nil { + t.Fatalf("deployment not created: %v", err) + } + return dep + } + + t.Run("no CA leaves no custom-ca volume", func(t *testing.T) { + dep := getDeployment(t, newRunner(nil)) + if hasVolume(dep, "custom-ca") { + t.Fatal("did not expect a custom-ca volume without a CA") + } + }) + + t.Run("configmap CA adds volume and read-only mount", func(t *testing.T) { + dep := getDeployment(t, newRunner(&v1beta2.CASource{ + ConfigMapKeyRef: &v1beta2.CAKeyRef{Name: "ca-cm"}, + })) + if !hasVolume(dep, "custom-ca") { + t.Fatal("expected a custom-ca volume") + } + if !hasMount(dep.Spec.Template.Spec.Containers[0], "custom-ca", types.CACertMountDir) { + t.Fatalf("expected custom-ca mounted read-only at %s", types.CACertMountDir) + } + }) + + t.Run("secret CA adds volume and read-only mount", func(t *testing.T) { + dep := getDeployment(t, newRunner(&v1beta2.CASource{ + SecretKeyRef: &v1beta2.CAKeyRef{Name: "ca-secret"}, + })) + if !hasVolume(dep, "custom-ca") { + t.Fatal("expected a custom-ca volume") + } + if !hasMount(dep.Spec.Template.Spec.Containers[0], "custom-ca", types.CACertMountDir) { + t.Fatalf("expected custom-ca mounted read-only at %s", types.CACertMountDir) + } + }) +} + +func hasVolume(dep appsv1.Deployment, name string) bool { + for _, v := range dep.Spec.Template.Spec.Volumes { + if v.Name == name { + return true + } + } + return false +} + +func hasMount(c corev1.Container, name, path string) bool { + for _, m := range c.VolumeMounts { + if m.Name == name && m.MountPath == path && m.ReadOnly { + return true + } + } + return false +} From ad3e8d50b5a84d91c7532cc2237aa70982ac8754 Mon Sep 17 00:00:00 2001 From: Alexander Chernov Date: Fri, 26 Jun 2026 00:50:35 +0100 Subject: [PATCH 2/3] feat(ca): allow an inline CA bundle in spec.caCertificate Add a value field to caCertificate so a PEM bundle can be given directly in the manifest, alongside the existing secretKeyRef / configMapKeyRef. The resolved CA (from any of the three sources) is now written into the runner's config Secret under ca.crt and mounted via the existing config volume, rather than mounting the source object as a separate volume. This unifies the three sources behind one tls-ca-file path and is what makes an inline value work, since there is no object to project for it. The admission webhook rejects setting more than one of value/secretKeyRef/configMapKeyRef. CRDs, README, sample, and tests updated. Refs: #21 Signed-off-by: Alexander Chernov --- README.md | 25 ++++-- api/v1beta2/ca_types.go | 35 ++++++-- api/v1beta2/multirunner_types.go | 3 +- api/v1beta2/runner_types.go | 2 +- .../gitlab.k8s.alekc.dev_multirunners.yaml | 9 +- .../bases/gitlab.k8s.alekc.dev_runners.yaml | 8 +- internal/controller/multirunner_controller.go | 12 ++- internal/controller/resolve_ca_test.go | 9 +- internal/controller/runner_controller.go | 12 ++- internal/controller/shared.go | 3 + internal/generate/config_test.go | 11 +++ internal/types/const.go | 13 +-- internal/validate/validate.go | 56 ++----------- internal/validate/validate_ca_test.go | 84 ++++++------------- 14 files changed, 145 insertions(+), 137 deletions(-) diff --git a/README.md b/README.md index e054dd0..0be183d 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,7 @@ Top-level `spec` fields (all optional unless noted): | `log_level` | One of `panic`, `fatal`, `error`, `warning`, `info`, `debug`. | | `log_format` | One of `runner`, `text`, `json`. | | `gitlab_instance_url` | GitLab URL. Defaults to `https://gitlab.com/`. | -| `caCertificate` | Optional PEM CA bundle to verify a private or self-signed GitLab endpoint, used for both the operator's API calls and the runner's own connection. References a `secretKeyRef` or `configMapKeyRef` (see below). | +| `caCertificate` | Optional PEM CA bundle to verify a private or self-signed GitLab endpoint, used for both the operator's API calls and the runner's own connection. Supply it inline (`value`) or from a `secretKeyRef` / `configMapKeyRef` (see below). | | `executor_config` | Kubernetes executor options, see the [keywords reference](https://docs.gitlab.com/runner/executors/kubernetes.html#configuration-settings). | | `environment` | Custom environment variables injected into the build environment. | | `runner_image` | Override the gitlab-runner image. Defaults to a recent `gitlab/gitlab-runner:alpine-vX.Y.Z`. | @@ -93,11 +93,13 @@ exclusive ways to supply the value: ### `caCertificate` fields -Set at most one reference. The named object must live in the runner namespace -and hold a PEM CA bundle. +Set at most one of the following. A referenced object must live in the runner +namespace and hold a PEM CA bundle. The operator copies the resolved bundle into +the runner's config Secret and points `tls-ca-file` at it. | Key | Description | | --- | --- | +| `value` | The PEM CA bundle inline, supplied directly in the manifest. | | `secretKeyRef` | Read the CA from a Secret: `name` (required), `key` (optional, defaults to `ca.crt`). | | `configMapKeyRef` | Read the CA from a ConfigMap: `name` (required), `key` (optional, defaults to `ca.crt`). | @@ -192,10 +194,10 @@ example that mixes both authentication modes across entries. ### Custom CA for a self-signed GitLab -Point `caCertificate` at a Secret or ConfigMap holding the PEM CA bundle. The +Set `caCertificate` to a PEM bundle, inline or from a Secret / ConfigMap. The operator uses it for its own API calls (fixing `x509: certificate signed by -unknown authority` during registration) and mounts it into the runner so its -jobs trust the endpoint too. +unknown authority` during registration) and copies it into the runner's config +Secret so the runner trusts the endpoint too. ```yaml apiVersion: gitlab.k8s.alekc.dev/v1beta2 @@ -217,6 +219,17 @@ spec: project_id: 1234567 ``` +Or supply the bundle inline with `value`: + +```yaml +spec: + caCertificate: + value: | + -----BEGIN CERTIFICATE----- + ...your CA here... + -----END CERTIFICATE----- +``` + ## RBAC and namespaces The kubernetes executor permission set (pods and pods/exec, pods/attach, diff --git a/api/v1beta2/ca_types.go b/api/v1beta2/ca_types.go index b990a98..5fa8206 100644 --- a/api/v1beta2/ca_types.go +++ b/api/v1beta2/ca_types.go @@ -7,10 +7,16 @@ import "fmt" // Secrets and the cluster root-CA ConfigMap. const DefaultCAKey = "ca.crt" -// CASource references a PEM-encoded CA bundle used to verify the GitLab -// endpoint, both for the operator's own API calls and for the runner's -// connection. Exactly one of SecretKeyRef or ConfigMapKeyRef may be set. +// CASource provides a PEM-encoded CA bundle used to verify the GitLab endpoint, +// both for the operator's own API calls and for the runner's connection. Set at +// most one of Value, SecretKeyRef, or ConfigMapKeyRef. type CASource struct { + // Value is an inline PEM CA bundle, supplied directly in the manifest. + // Convenient for small bundles; prefer a Secret or ConfigMap ref when the + // bundle is large or rotated independently of the runner spec. + // +optional + Value string `json:"value,omitempty"` + // SecretKeyRef selects a key in a Secret holding the PEM CA bundle. // +optional SecretKeyRef *CAKeyRef `json:"secretKeyRef,omitempty"` @@ -30,19 +36,30 @@ type CAKeyRef struct { Key string `json:"key,omitempty"` } -// IsSet reports whether the source selects a CA bundle. +// IsSet reports whether the source provides a CA bundle. func (c *CASource) IsSet() bool { - return c != nil && (c.SecretKeyRef != nil || c.ConfigMapKeyRef != nil) + return c != nil && (c.Value != "" || c.SecretKeyRef != nil || c.ConfigMapKeyRef != nil) } -// Validate enforces that at most one ref is set and that a set ref names a -// source. A nil source is valid (no custom CA). +// Validate enforces that at most one of value, secretKeyRef, or configMapKeyRef +// is set and that a set ref names a source. A nil source is valid (no custom +// CA). func (c *CASource) Validate() error { if c == nil { return nil } - if c.SecretKeyRef != nil && c.ConfigMapKeyRef != nil { - return fmt.Errorf("caCertificate: set only one of secretKeyRef or configMapKeyRef") + set := 0 + if c.Value != "" { + set++ + } + if c.SecretKeyRef != nil { + set++ + } + if c.ConfigMapKeyRef != nil { + set++ + } + if set > 1 { + return fmt.Errorf("caCertificate: set only one of value, secretKeyRef, or configMapKeyRef") } if c.SecretKeyRef != nil && c.SecretKeyRef.Name == "" { return fmt.Errorf("caCertificate.secretKeyRef.name is required") diff --git a/api/v1beta2/multirunner_types.go b/api/v1beta2/multirunner_types.go index 4ff4499..05552a8 100644 --- a/api/v1beta2/multirunner_types.go +++ b/api/v1beta2/multirunner_types.go @@ -69,7 +69,8 @@ type MultiRunnerSpec struct { // CACertificate, when set, provides a PEM CA bundle used to verify the // GitLab endpoint for both the operator's API calls and every runner - // entry's own connection. Reference it from a Secret or a ConfigMap key. + // entry's own connection. Supply it inline (value) or from a Secret or + // ConfigMap key. // +optional CACertificate *CASource `json:"caCertificate,omitempty"` diff --git a/api/v1beta2/runner_types.go b/api/v1beta2/runner_types.go index 68e5bac..09714c9 100644 --- a/api/v1beta2/runner_types.go +++ b/api/v1beta2/runner_types.go @@ -79,7 +79,7 @@ type RunnerSpec struct { // CACertificate, when set, provides a PEM CA bundle used to verify the // GitLab endpoint for both the operator's API calls and the runner's own - // connection. Reference it from a Secret or a ConfigMap key. + // connection. Supply it inline (value) or from a Secret/ConfigMap key. // +optional CACertificate *CASource `json:"caCertificate,omitempty"` } diff --git a/config/crd/bases/gitlab.k8s.alekc.dev_multirunners.yaml b/config/crd/bases/gitlab.k8s.alekc.dev_multirunners.yaml index 6f3ed1e..ea6800b 100644 --- a/config/crd/bases/gitlab.k8s.alekc.dev_multirunners.yaml +++ b/config/crd/bases/gitlab.k8s.alekc.dev_multirunners.yaml @@ -50,7 +50,8 @@ spec: description: |- CACertificate, when set, provides a PEM CA bundle used to verify the GitLab endpoint for both the operator's API calls and every runner - entry's own connection. Reference it from a Secret or a ConfigMap key. + entry's own connection. Supply it inline (value) or from a Secret or + ConfigMap key. properties: configMapKeyRef: description: ConfigMapKeyRef selects a key in a ConfigMap holding @@ -80,6 +81,12 @@ spec: required: - name type: object + value: + description: |- + Value is an inline PEM CA bundle, supplied directly in the manifest. + Convenient for small bundles; prefer a Secret or ConfigMap ref when the + bundle is large or rotated independently of the runner spec. + type: string type: object check_interval: minimum: 3 diff --git a/config/crd/bases/gitlab.k8s.alekc.dev_runners.yaml b/config/crd/bases/gitlab.k8s.alekc.dev_runners.yaml index bcc471a..a175557 100644 --- a/config/crd/bases/gitlab.k8s.alekc.dev_runners.yaml +++ b/config/crd/bases/gitlab.k8s.alekc.dev_runners.yaml @@ -161,7 +161,7 @@ spec: description: |- CACertificate, when set, provides a PEM CA bundle used to verify the GitLab endpoint for both the operator's API calls and the runner's own - connection. Reference it from a Secret or a ConfigMap key. + connection. Supply it inline (value) or from a Secret/ConfigMap key. properties: configMapKeyRef: description: ConfigMapKeyRef selects a key in a ConfigMap holding @@ -191,6 +191,12 @@ spec: required: - name type: object + value: + description: |- + Value is an inline PEM CA bundle, supplied directly in the manifest. + Convenient for small bundles; prefer a Secret or ConfigMap ref when the + bundle is large or rotated independently of the runner spec. + type: string type: object check_interval: minimum: 3 diff --git a/internal/controller/multirunner_controller.go b/internal/controller/multirunner_controller.go index 7f05114..96fd450 100644 --- a/internal/controller/multirunner_controller.go +++ b/internal/controller/multirunner_controller.go @@ -153,8 +153,16 @@ func (r *MultiRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Request) // leaves the runner stuck below Ready. runnerObj.SetConfigMapVersion(configHashKey) - // reconcile the config Secret (config.toml plus the per-entry tokens) - if res, err := validate.Secret(ctx, r.Client, runnerObj, logger, generatedTomlConfig, tokens); res != nil || err != nil { + // resolve the custom CA bundle (inline value, Secret, or ConfigMap) so it is + // stored in the config Secret and trusted by the runner pod. + caPEM, err := resolveCABundle(ctx, r.Client, runnerObj.GetNamespace(), runnerObj.CACertificate()) + if err != nil { + runnerObj.SetReadyCondition(false, "CAResolveFailed", err.Error()) + return resultRequeueAfterDefaultTimeout, err + } + + // reconcile the config Secret (config.toml, the per-entry tokens, and the CA) + if res, err := validate.Secret(ctx, r.Client, runnerObj, logger, generatedTomlConfig, tokens, caPEM); res != nil || err != nil { if err != nil { runnerObj.SetReadyCondition(false, "ConfigSecretFailed", err.Error()) } diff --git a/internal/controller/resolve_ca_test.go b/internal/controller/resolve_ca_test.go index fad4a52..4b504ee 100644 --- a/internal/controller/resolve_ca_test.go +++ b/internal/controller/resolve_ca_test.go @@ -42,6 +42,7 @@ func TestResolveCABundle(t *testing.T) { }{ {name: "nil source", src: nil, want: ""}, {name: "empty source", src: &v1beta2.CASource{}, want: ""}, + {name: "inline value", src: &v1beta2.CASource{Value: "inline-ca-pem"}, want: "inline-ca-pem"}, { name: "secret default key", src: &v1beta2.CASource{SecretKeyRef: &v1beta2.CAKeyRef{Name: "ca-secret"}}, @@ -117,10 +118,16 @@ func TestCASourceValidate(t *testing.T) { {name: "secret no name", src: &v1beta2.CASource{SecretKeyRef: &v1beta2.CAKeyRef{}}, wantErr: true}, {name: "configmap no name", src: &v1beta2.CASource{ConfigMapKeyRef: &v1beta2.CAKeyRef{}}, wantErr: true}, { - name: "both set", + name: "both refs set", src: &v1beta2.CASource{SecretKeyRef: &v1beta2.CAKeyRef{Name: "s"}, ConfigMapKeyRef: &v1beta2.CAKeyRef{Name: "c"}}, wantErr: true, }, + {name: "inline value ok", src: &v1beta2.CASource{Value: "x"}}, + { + name: "value plus secret ref", + src: &v1beta2.CASource{Value: "x", SecretKeyRef: &v1beta2.CAKeyRef{Name: "s"}}, + wantErr: true, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/internal/controller/runner_controller.go b/internal/controller/runner_controller.go index b134bb9..bb4d363 100644 --- a/internal/controller/runner_controller.go +++ b/internal/controller/runner_controller.go @@ -174,8 +174,16 @@ func (r *RunnerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // leaves the runner stuck below Ready. runnerObj.SetConfigMapVersion(configHashKey) - // reconcile the config Secret (config.toml plus the per-entry tokens) - if res, err := validate.Secret(ctx, r.Client, runnerObj, logger, generatedTomlConfig, tokens); res != nil || err != nil { + // resolve the custom CA bundle (inline value, Secret, or ConfigMap) so it is + // stored in the config Secret and trusted by the runner pod. + caPEM, err := resolveCABundle(ctx, r.Client, runnerObj.GetNamespace(), runnerObj.CACertificate()) + if err != nil { + runnerObj.SetReadyCondition(false, "CAResolveFailed", err.Error()) + return resultRequeueAfterDefaultTimeout, err + } + + // reconcile the config Secret (config.toml, the per-entry tokens, and the CA) + if res, err := validate.Secret(ctx, r.Client, runnerObj, logger, generatedTomlConfig, tokens, caPEM); res != nil || err != nil { if err != nil { runnerObj.SetReadyCondition(false, "ConfigSecretFailed", err.Error()) } diff --git a/internal/controller/shared.go b/internal/controller/shared.go index 96ed0d9..dfb4a7a 100644 --- a/internal/controller/shared.go +++ b/internal/controller/shared.go @@ -100,6 +100,9 @@ func resolveCABundle(ctx context.Context, cl client.Client, namespace string, sr if !src.IsSet() { return nil, nil } + if src.Value != "" { + return []byte(src.Value), nil + } switch { case src.SecretKeyRef != nil: ref := src.SecretKeyRef diff --git a/internal/generate/config_test.go b/internal/generate/config_test.go index 22aca05..a6ee5fe 100644 --- a/internal/generate/config_test.go +++ b/internal/generate/config_test.go @@ -37,6 +37,17 @@ func TestSingleRunnerConfig_TLSCAFile(t *testing.T) { if !strings.Contains(cfg, wantTLSCALine()) { t.Fatalf("expected %q in config, got:\n%s", wantTLSCALine(), cfg) } + + // an inline value sets tls-ca-file the same way as a ref + inline := base.DeepCopy() + inline.Spec.CACertificate = &v1beta2.CASource{Value: "-----BEGIN CERTIFICATE-----\nx\n-----END CERTIFICATE-----\n"} + cfg, _, err = SingleRunnerConfig(inline, tokens) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(cfg, wantTLSCALine()) { + t.Fatalf("expected %q for an inline CA, got:\n%s", wantTLSCALine(), cfg) + } } func TestMultiRunnerConfig_TLSCAFile(t *testing.T) { diff --git a/internal/types/const.go b/internal/types/const.go index 11dad47..761c40c 100644 --- a/internal/types/const.go +++ b/internal/types/const.go @@ -9,12 +9,13 @@ const ConfigMapKeyName = "config.toml" // live in the CR status. const ConfigTokenKeyPrefix = "authentication-token-" -// CACertMountDir is where a custom CA bundle is mounted into the runner pod and -// CACertFileName is the file it is projected to. CACertFile is the absolute -// path written into config.toml as tls-ca-file. The directory sits outside the -// config-Secret mount (/etc/gitlab-runner) to avoid nested mounts. +// CACertFileName is the config-Secret data key (and projected filename) for a +// custom CA bundle; CACertFile is its absolute path inside the runner container, +// written into config.toml as tls-ca-file. The CA is stored in the config Secret +// alongside config.toml, which is mounted at /etc/gitlab-runner, so no extra +// volume is needed. Keep this prefix in sync with the config volume mount path +// in validate.Deployment. const ( - CACertMountDir = "/etc/gitlab-certs" CACertFileName = "ca.crt" - CACertFile = CACertMountDir + "/" + CACertFileName + CACertFile = "/etc/gitlab-runner/" + CACertFileName ) diff --git a/internal/validate/validate.go b/internal/validate/validate.go index c516810..dc579c8 100644 --- a/internal/validate/validate.go +++ b/internal/validate/validate.go @@ -5,7 +5,6 @@ import ( "reflect" "github.com/go-logr/logr" - "gitlab.k8s.alekc.dev/api/v1beta2" "gitlab.k8s.alekc.dev/internal/result" "gitlab.k8s.alekc.dev/internal/types" appsv1 "k8s.io/api/apps/v1" @@ -85,13 +84,6 @@ func Deployment(ctx context.Context, cl client.Client, runnerObj types.RunnerInf }, } - // Mount a custom CA bundle into the runner container when configured so the - // runner trusts a private or self-signed GitLab endpoint (config.toml points - // at it via tls-ca-file). - if src := runnerObj.CACertificate(); src.IsSet() { - addCustomCAVolume(&wantedDeployment, src) - } - var existingDeployment appsv1.Deployment err := cl.Get(ctx, client.ObjectKey{ Namespace: runnerObj.GetNamespace(), @@ -138,58 +130,22 @@ func Deployment(ctx context.Context, cl client.Client, runnerObj types.RunnerInf return nil, nil } -// addCustomCAVolume mounts the referenced CA bundle (from a Secret or a -// ConfigMap key) into the runner container at types.CACertFile, read-only, so -// config.toml's tls-ca-file can point at it. The chosen key is projected to a -// fixed filename so the mount path is stable regardless of the source key. -func addCustomCAVolume(dep *appsv1.Deployment, src *v1beta2.CASource) { - const volName = "custom-ca" - var vs corev1.VolumeSource - switch { - case src.SecretKeyRef != nil: - key := src.SecretKeyRef.Key - if key == "" { - key = v1beta2.DefaultCAKey - } - vs.Secret = &corev1.SecretVolumeSource{ - SecretName: src.SecretKeyRef.Name, - Items: []corev1.KeyToPath{{Key: key, Path: types.CACertFileName}}, - } - case src.ConfigMapKeyRef != nil: - key := src.ConfigMapKeyRef.Key - if key == "" { - key = v1beta2.DefaultCAKey - } - vs.ConfigMap = &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{Name: src.ConfigMapKeyRef.Name}, - Items: []corev1.KeyToPath{{Key: key, Path: types.CACertFileName}}, - } - default: - return - } - dep.Spec.Template.Spec.Volumes = append(dep.Spec.Template.Spec.Volumes, corev1.Volume{ - Name: volName, - VolumeSource: vs, - }) - c := &dep.Spec.Template.Spec.Containers[0] - c.VolumeMounts = append(c.VolumeMounts, corev1.VolumeMount{ - Name: volName, - MountPath: types.CACertMountDir, - ReadOnly: true, - }) -} - // Secret renders the runner config.toml into an Opaque Secret named after the // runner, keeping each entry's authentication token under a dedicated // ConfigTokenKeyPrefix key so the controller can recover it on later // reconciles. The token never lands in a ConfigMap or in the CR status. -func Secret(ctx context.Context, cl client.Client, runnerObj types.RunnerInfo, logger logr.Logger, gitlabRunnerTomlConfig string, tokens map[string]string) (*ctrl.Result, error) { +func Secret(ctx context.Context, cl client.Client, runnerObj types.RunnerInfo, logger logr.Logger, gitlabRunnerTomlConfig string, tokens map[string]string, caPEM []byte) (*ctrl.Result, error) { desired := map[string][]byte{ types.ConfigMapKeyName: []byte(gitlabRunnerTomlConfig), } for name, token := range tokens { desired[types.ConfigTokenKeyPrefix+name] = []byte(token) } + // When a custom CA is configured, store it alongside config.toml so it is + // mounted into the runner at types.CACertFile (config.toml's tls-ca-file). + if len(caPEM) > 0 { + desired[types.CACertFileName] = caPEM + } var secret corev1.Secret err := cl.Get(ctx, client.ObjectKey{ diff --git a/internal/validate/validate_ca_test.go b/internal/validate/validate_ca_test.go index a6360e6..3133b57 100644 --- a/internal/validate/validate_ca_test.go +++ b/internal/validate/validate_ca_test.go @@ -7,7 +7,6 @@ import ( "github.com/go-logr/logr" "gitlab.k8s.alekc.dev/api/v1beta2" "gitlab.k8s.alekc.dev/internal/types" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -16,7 +15,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" ) -func TestDeployment_CustomCAMount(t *testing.T) { +// TestSecret_CAKey verifies the resolved CA bundle is written into the config +// Secret under types.CACertFileName (so it is mounted into the runner via the +// existing config volume), and is absent when no CA is configured. +func TestSecret_CAKey(t *testing.T) { scheme := runtime.NewScheme() if err := clientgoscheme.AddToScheme(scheme); err != nil { t.Fatal(err) @@ -25,74 +27,42 @@ func TestDeployment_CustomCAMount(t *testing.T) { t.Fatal(err) } - newRunner := func(ca *v1beta2.CASource) *v1beta2.Runner { - return &v1beta2.Runner{ - TypeMeta: metav1.TypeMeta{APIVersion: "gitlab.k8s.alekc.dev/v1beta2", Kind: "Runner"}, - ObjectMeta: metav1.ObjectMeta{Name: "r1", Namespace: "ns", UID: "uid-1"}, - Spec: v1beta2.RunnerSpec{CACertificate: ca}, - } + runner := &v1beta2.Runner{ + TypeMeta: metav1.TypeMeta{APIVersion: "gitlab.k8s.alekc.dev/v1beta2", Kind: "Runner"}, + ObjectMeta: metav1.ObjectMeta{Name: "r1", Namespace: "ns", UID: "uid-1"}, } - getDeployment := func(t *testing.T, runner *v1beta2.Runner) appsv1.Deployment { + getSecret := func(t *testing.T, caPEM []byte) corev1.Secret { t.Helper() cl := fake.NewClientBuilder().WithScheme(scheme).Build() - if _, err := Deployment(context.Background(), cl, runner, logr.Discard()); err != nil { - t.Fatalf("Deployment: %v", err) + _, err := Secret(context.Background(), cl, runner, logr.Discard(), "[[runners]]\n", map[string]string{"r1": "glrt-x"}, caPEM) + if err != nil { + t.Fatalf("Secret: %v", err) } - var dep appsv1.Deployment + var s corev1.Secret key := client.ObjectKey{Namespace: "ns", Name: runner.ChildName()} - if err := cl.Get(context.Background(), key, &dep); err != nil { - t.Fatalf("deployment not created: %v", err) + if err := cl.Get(context.Background(), key, &s); err != nil { + t.Fatalf("config secret not created: %v", err) } - return dep + return s } - t.Run("no CA leaves no custom-ca volume", func(t *testing.T) { - dep := getDeployment(t, newRunner(nil)) - if hasVolume(dep, "custom-ca") { - t.Fatal("did not expect a custom-ca volume without a CA") + t.Run("no CA leaves no ca.crt key", func(t *testing.T) { + s := getSecret(t, nil) + if _, ok := s.Data[types.CACertFileName]; ok { + t.Fatal("did not expect a ca.crt key without a CA") } }) - t.Run("configmap CA adds volume and read-only mount", func(t *testing.T) { - dep := getDeployment(t, newRunner(&v1beta2.CASource{ - ConfigMapKeyRef: &v1beta2.CAKeyRef{Name: "ca-cm"}, - })) - if !hasVolume(dep, "custom-ca") { - t.Fatal("expected a custom-ca volume") + t.Run("CA present is stored under ca.crt", func(t *testing.T) { + pem := []byte("-----BEGIN CERTIFICATE-----\nMIIBfake\n-----END CERTIFICATE-----\n") + s := getSecret(t, pem) + got, ok := s.Data[types.CACertFileName] + if !ok { + t.Fatal("expected a ca.crt key in the config secret") } - if !hasMount(dep.Spec.Template.Spec.Containers[0], "custom-ca", types.CACertMountDir) { - t.Fatalf("expected custom-ca mounted read-only at %s", types.CACertMountDir) + if string(got) != string(pem) { + t.Fatalf("ca.crt = %q, want %q", got, pem) } }) - - t.Run("secret CA adds volume and read-only mount", func(t *testing.T) { - dep := getDeployment(t, newRunner(&v1beta2.CASource{ - SecretKeyRef: &v1beta2.CAKeyRef{Name: "ca-secret"}, - })) - if !hasVolume(dep, "custom-ca") { - t.Fatal("expected a custom-ca volume") - } - if !hasMount(dep.Spec.Template.Spec.Containers[0], "custom-ca", types.CACertMountDir) { - t.Fatalf("expected custom-ca mounted read-only at %s", types.CACertMountDir) - } - }) -} - -func hasVolume(dep appsv1.Deployment, name string) bool { - for _, v := range dep.Spec.Template.Spec.Volumes { - if v.Name == name { - return true - } - } - return false -} - -func hasMount(c corev1.Container, name, path string) bool { - for _, m := range c.VolumeMounts { - if m.Name == name && m.MountPath == path && m.ReadOnly { - return true - } - } - return false } From eb9f064a6a1d411728bd4240c1fe3a42f8610693 Mon Sep 17 00:00:00 2001 From: Alexander Chernov Date: Fri, 26 Jun 2026 10:18:42 +0100 Subject: [PATCH 3/3] fix(ca): address review feedback on the custom-CA path - Clone http.DefaultTransport when adding the custom CA so the operator's API client keeps proxy (HTTPS_PROXY), timeouts, and HTTP/2 rather than a bare Transport that drops them. - Resolve the CA once per reconcile via the uncached APIReader (ConfigMaps are not watched) and thread the bytes to the API client, the config hash, and the config Secret, removing the duplicate cached-client lookups. - Fold the CA bytes into configHashKey so a CA rotation rolls the deployment, since config.toml only references tls-ca-file by path. - During finalization, recover the CA from the persisted config Secret rather than the user's CA Secret/ConfigMap (which may already be gone), so the GitLab runner is still unregistered. Refs: #21 Signed-off-by: Alexander Chernov --- internal/api/gitlab.go | 11 +++-- internal/controller/multirunner_controller.go | 24 ++++++----- internal/controller/runner_controller.go | 24 ++++++----- internal/controller/shared.go | 33 +++++++------- internal/crud/crud.go | 16 +++++++ internal/generate/config.go | 28 +++++++++--- internal/generate/config_test.go | 43 ++++++++++++++++--- 7 files changed, 124 insertions(+), 55 deletions(-) diff --git a/internal/api/gitlab.go b/internal/api/gitlab.go index 2dc859c..537273a 100644 --- a/internal/api/gitlab.go +++ b/internal/api/gitlab.go @@ -159,12 +159,11 @@ func NewGitlabClient(token, url string, caPEM []byte) (GitlabClient, error) { if !pool.AppendCertsFromPEM(caPEM) { return nil, fmt.Errorf("custom CA bundle contains no valid PEM certificate") } - httpClient := &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{RootCAs: pool, MinVersion: tls.VersionTLS12}, - }, - } - opts = append(opts, gitlab.WithHTTPClient(httpClient)) + // Clone the default transport so proxy settings (HTTPS_PROXY), timeouts, + // and HTTP/2 are preserved; only the trust pool is overridden. + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.TLSClientConfig = &tls.Config{RootCAs: pool, MinVersion: tls.VersionTLS12} + opts = append(opts, gitlab.WithHTTPClient(&http.Client{Transport: transport})) } obj := &gitlabApi{} var err error diff --git a/internal/controller/multirunner_controller.go b/internal/controller/multirunner_controller.go index 96fd450..55e7bbb 100644 --- a/internal/controller/multirunner_controller.go +++ b/internal/controller/multirunner_controller.go @@ -118,9 +118,21 @@ func (r *MultiRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Request) runnerObj.SetStatusReady(false) runnerObj.SetObservedGeneration(runnerObj.GetGeneration()) + // resolve the custom CA bundle once (inline value, Secret, or ConfigMap) via + // the uncached APIReader since ConfigMaps are not watched. It is used for the + // API client, folded into the config hash, and stored in the config Secret so + // the runner pod trusts the endpoint too. + caPEM, err := resolveCABundle(ctx, r.APIReader, runnerObj.GetNamespace(), runnerObj.CACertificate()) + if err != nil { + runnerObj.SetStatusError(err.Error()) + runnerObj.SetReadyCondition(false, "CAResolveFailed", err.Error()) + logger.Error(err, "cannot resolve the custom CA bundle") + return resultRequeueAfterDefaultTimeout, err + } + // resolve auth and ensure managed runners exist on GitLab. Managed runner // ids are persisted to status immediately inside ensureRunners. - tokens, requeueAfter, err := ensureRunners(ctx, r.Client, r.Status(), r.GitlabApiClient, runnerObj, logger) + tokens, requeueAfter, err := ensureRunners(ctx, r.Client, r.Status(), r.GitlabApiClient, runnerObj, caPEM, logger) if err != nil { runnerObj.SetStatusError(err.Error()) runnerObj.SetReadyCondition(false, "AuthFailed", err.Error()) @@ -137,7 +149,7 @@ func (r *MultiRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Request) } // render config.toml from the resolved tokens - generatedTomlConfig, configHashKey, err := generate.TomlConfig(runnerObj, tokens) + generatedTomlConfig, configHashKey, err := generate.TomlConfig(runnerObj, tokens, caPEM) if err != nil { runnerObj.SetStatusError(err.Error()) runnerObj.SetReadyCondition(false, "ConfigRenderFailed", err.Error()) @@ -153,14 +165,6 @@ func (r *MultiRunnerReconciler) Reconcile(ctx context.Context, req ctrl.Request) // leaves the runner stuck below Ready. runnerObj.SetConfigMapVersion(configHashKey) - // resolve the custom CA bundle (inline value, Secret, or ConfigMap) so it is - // stored in the config Secret and trusted by the runner pod. - caPEM, err := resolveCABundle(ctx, r.Client, runnerObj.GetNamespace(), runnerObj.CACertificate()) - if err != nil { - runnerObj.SetReadyCondition(false, "CAResolveFailed", err.Error()) - return resultRequeueAfterDefaultTimeout, err - } - // reconcile the config Secret (config.toml, the per-entry tokens, and the CA) if res, err := validate.Secret(ctx, r.Client, runnerObj, logger, generatedTomlConfig, tokens, caPEM); res != nil || err != nil { if err != nil { diff --git a/internal/controller/runner_controller.go b/internal/controller/runner_controller.go index bb4d363..0f20fdb 100644 --- a/internal/controller/runner_controller.go +++ b/internal/controller/runner_controller.go @@ -139,9 +139,21 @@ func (r *RunnerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr runnerObj.SetStatusReady(false) runnerObj.SetObservedGeneration(runnerObj.GetGeneration()) + // resolve the custom CA bundle once (inline value, Secret, or ConfigMap) via + // the uncached APIReader since ConfigMaps are not watched. It is used for the + // API client, folded into the config hash, and stored in the config Secret so + // the runner pod trusts the endpoint too. + caPEM, err := resolveCABundle(ctx, r.APIReader, runnerObj.GetNamespace(), runnerObj.CACertificate()) + if err != nil { + runnerObj.SetStatusError(err.Error()) + runnerObj.SetReadyCondition(false, "CAResolveFailed", err.Error()) + logger.Error(err, "cannot resolve the custom CA bundle") + return resultRequeueAfterDefaultTimeout, err + } + // resolve auth and ensure managed runners exist on GitLab. Managed runner // ids are persisted to status immediately inside ensureRunners. - tokens, requeueAfter, err := ensureRunners(ctx, r.Client, r.Status(), r.GitlabApiClient, runnerObj, logger) + tokens, requeueAfter, err := ensureRunners(ctx, r.Client, r.Status(), r.GitlabApiClient, runnerObj, caPEM, logger) if err != nil { runnerObj.SetStatusError(err.Error()) runnerObj.SetReadyCondition(false, "AuthFailed", err.Error()) @@ -158,7 +170,7 @@ func (r *RunnerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } // render config.toml from the resolved tokens - generatedTomlConfig, configHashKey, err := generate.TomlConfig(runnerObj, tokens) + generatedTomlConfig, configHashKey, err := generate.TomlConfig(runnerObj, tokens, caPEM) if err != nil { runnerObj.SetStatusError(err.Error()) runnerObj.SetReadyCondition(false, "ConfigRenderFailed", err.Error()) @@ -174,14 +186,6 @@ func (r *RunnerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // leaves the runner stuck below Ready. runnerObj.SetConfigMapVersion(configHashKey) - // resolve the custom CA bundle (inline value, Secret, or ConfigMap) so it is - // stored in the config Secret and trusted by the runner pod. - caPEM, err := resolveCABundle(ctx, r.Client, runnerObj.GetNamespace(), runnerObj.CACertificate()) - if err != nil { - runnerObj.SetReadyCondition(false, "CAResolveFailed", err.Error()) - return resultRequeueAfterDefaultTimeout, err - } - // reconcile the config Secret (config.toml, the per-entry tokens, and the CA) if res, err := validate.Secret(ctx, r.Client, runnerObj, logger, generatedTomlConfig, tokens, caPEM); res != nil || err != nil { if err != nil { diff --git a/internal/controller/shared.go b/internal/controller/shared.go index dfb4a7a..bbd3b80 100644 --- a/internal/controller/shared.go +++ b/internal/controller/shared.go @@ -93,10 +93,11 @@ func resolveTokenSource(ctx context.Context, cl client.Client, namespace string, return string(value), nil } -// resolveCABundle resolves a CASource to its PEM bytes, read from the named key -// (defaulting to v1beta2.DefaultCAKey) of a Secret or ConfigMap in namespace. A -// nil or empty source returns nil with no error. -func resolveCABundle(ctx context.Context, cl client.Client, namespace string, src *v1beta2.CASource) ([]byte, error) { +// resolveCABundle resolves a CASource to its PEM bytes: an inline value, or the +// named key (defaulting to v1beta2.DefaultCAKey) of a Secret or ConfigMap in +// namespace. It takes a client.Reader so callers can pass the uncached APIReader +// (ConfigMaps are not watched). A nil or empty source returns nil with no error. +func resolveCABundle(ctx context.Context, cl client.Reader, namespace string, src *v1beta2.CASource) ([]byte, error) { if !src.IsSet() { return nil, nil } @@ -145,7 +146,7 @@ func resolveCABundle(ctx context.Context, cl client.Client, namespace string, sr // Secret). It also returns a RequeueAfter so managed-runner token expiry is // re-checked even without a spec change. The token is never stored in status; // managed tokens are persisted in the config Secret and recovered from there. -func ensureRunners(ctx context.Context, cl client.Client, statusW client.StatusWriter, injected api.GitlabClient, obj types.RunnerInfo, logger logr.Logger) (map[string]string, time.Duration, error) { +func ensureRunners(ctx context.Context, cl client.Client, statusW client.StatusWriter, injected api.GitlabClient, obj types.RunnerInfo, caPEM []byte, logger logr.Logger) (map[string]string, time.Duration, error) { namespace := obj.GetNamespace() existing, err := crud.ExistingConfigTokens(ctx, cl, namespace, obj.ChildName()) if err != nil { @@ -169,7 +170,7 @@ func ensureRunners(ctx context.Context, cl client.Client, statusW client.StatusW continue } - token, expiry, err := ensureManagedRunner(ctx, cl, statusW, injected, obj, ®, existing[reg.Name], logger) + token, expiry, err := ensureManagedRunner(ctx, cl, statusW, injected, obj, ®, existing[reg.Name], caPEM, logger) if err != nil { return nil, 0, err } @@ -200,7 +201,7 @@ func resyncAfter(soonestExpiry *time.Time) time.Duration { // ensureManagedRunner reconciles a single managed runner against GitLab and // returns its current authentication token and expiry. -func ensureManagedRunner(ctx context.Context, cl client.Client, statusW client.StatusWriter, injected api.GitlabClient, obj types.RunnerInfo, reg *v1beta2.GitlabRegInfo, recoveredToken string, logger logr.Logger) (string, *time.Time, error) { +func ensureManagedRunner(ctx context.Context, cl client.Client, statusW client.StatusWriter, injected api.GitlabClient, obj types.RunnerInfo, reg *v1beta2.GitlabRegInfo, recoveredToken string, caPEM []byte, logger logr.Logger) (string, *time.Time, error) { desiredHash := reg.Auth.CreateOptions.Hash() gitlabClient := func() (api.GitlabClient, error) { @@ -214,10 +215,6 @@ func ensureManagedRunner(ctx context.Context, cl client.Client, statusW client.S if accessToken == "" { return nil, fmt.Errorf("managed runner %q requires an access_token (value or secret_key_ref)", reg.Name) } - caPEM, err := resolveCABundle(ctx, cl, obj.GetNamespace(), reg.CACertificate) - if err != nil { - return nil, err - } return api.NewGitlabClient(accessToken, reg.GitlabUrl, caPEM) } @@ -341,12 +338,18 @@ func removeManagedRunners(ctx context.Context, cl client.Client, injected api.Gi if err != nil { return fmt.Errorf("cannot read config secret to recover runner tokens: %w", err) } + // Recover the CA from the persisted config Secret rather than re-resolving + // the user's CA Secret/ConfigMap, which may already be gone at finalization. + caPEM, err := crud.ExistingConfigCA(ctx, cl, obj.GetNamespace(), obj.ChildName()) + if err != nil { + return fmt.Errorf("cannot read config secret to recover the CA bundle: %w", err) + } var firstErr error for _, reg := range obj.RegistrationConfig() { if !reg.Auth.IsManaged() || reg.RunnerID == 0 { continue } - if err := deleteManagedRunner(ctx, cl, injected, obj.GetNamespace(), reg, tokens[reg.Name], logger); err != nil { + if err := deleteManagedRunner(ctx, cl, injected, obj.GetNamespace(), reg, tokens[reg.Name], caPEM, logger); err != nil { logger.Error(err, "cannot delete runner from gitlab", "name", reg.Name, "id", reg.RunnerID) if firstErr == nil { firstErr = err @@ -362,11 +365,7 @@ func removeManagedRunners(ctx context.Context, cl client.Client, injected api.Gi // /runners/:id, which needs the api scope) when the token is missing or the // token-based delete is rejected. When neither path succeeds the runner may be // left orphaned and an error is returned so the finalizer can retry. -func deleteManagedRunner(ctx context.Context, cl client.Client, injected api.GitlabClient, namespace string, reg v1beta2.GitlabRegInfo, token string, logger logr.Logger) error { - caPEM, err := resolveCABundle(ctx, cl, namespace, reg.CACertificate) - if err != nil { - return err - } +func deleteManagedRunner(ctx context.Context, cl client.Client, injected api.GitlabClient, namespace string, reg v1beta2.GitlabRegInfo, token string, caPEM []byte, logger logr.Logger) error { // Preferred path: delete by the runner's own token (no access-token scope). if token != "" { gc := injected diff --git a/internal/crud/crud.go b/internal/crud/crud.go index 7057481..e362e17 100644 --- a/internal/crud/crud.go +++ b/internal/crud/crud.go @@ -80,6 +80,22 @@ func ExistingConfigTokens(ctx context.Context, cl client.Client, namespace, chil return out, nil } +// ExistingConfigCA returns the custom CA bundle persisted in the runner's config +// Secret (the CACertFileName key), or nil when the Secret or key is absent. The +// delete path uses it so unregistration does not depend on the user's CA +// Secret/ConfigMap, which may already be gone at finalization. +func ExistingConfigCA(ctx context.Context, cl client.Client, namespace, childName string) ([]byte, error) { + var secret corev1.Secret + err := cl.Get(ctx, client.ObjectKey{Namespace: namespace, Name: childName}, &secret) + if errors.IsNotFound(err) { + return nil, nil + } + if err != nil { + return nil, err + } + return secret.Data[internalTypes.CACertFileName], nil +} + // CreateRBACIfMissing reconciles the runner's RBAC. The permission set lives in // one shared ClusterRole; each runner gets its own ServiceAccount (distinct // identity, audit, and lifecycle) and a RoleBinding in every namespace its diff --git a/internal/generate/config.go b/internal/generate/config.go index 1930612..7292f6b 100644 --- a/internal/generate/config.go +++ b/internal/generate/config.go @@ -15,18 +15,18 @@ import ( // the resolved authentication tokens keyed by runner/entry name (the token is // never read from status; the caller supplies it from the create/refresh result // or the existing config Secret). -func TomlConfig(runner types.RunnerInfo, tokens map[string]string) (gitlabConfig, configHashKey string, err error) { +func TomlConfig(runner types.RunnerInfo, tokens map[string]string, caPEM []byte) (gitlabConfig, configHashKey string, err error) { // ugly as hell, but its the best I can do for now to avoid the import loop. // Blame the kubebuilder which cannot generate deepCopy for external workspace switch r := runner.(type) { case *v1beta2.Runner: - return SingleRunnerConfig(r, tokens) + return SingleRunnerConfig(r, tokens, caPEM) case *v1beta2.MultiRunner: - return MultiRunnerConfig(r, tokens) + return MultiRunnerConfig(r, tokens, caPEM) } panic("unknown runner type") } -func SingleRunnerConfig(r *v1beta2.Runner, tokens map[string]string) (gitlabConfig, configHashKey string, err error) { +func SingleRunnerConfig(r *v1beta2.Runner, tokens map[string]string, caPEM []byte) (gitlabConfig, configHashKey string, err error) { // define sensible config for some configuration values runnerConfig := &config.RunnerConfig{ Name: r.Name, @@ -69,12 +69,19 @@ func SingleRunnerConfig(r *v1beta2.Runner, tokens map[string]string) (gitlabConf } gitlabConfig = buff.String() - configHashKey = crypto.StringToSHA1(gitlabConfig) + // Fold the CA bundle into the hash so a CA content change (rotation) bumps + // the config version and rolls the deployment, even though config.toml only + // references tls-ca-file by path. Empty caPEM keeps the no-CA hash unchanged. + hashInput := gitlabConfig + if len(caPEM) > 0 { + hashInput += "\n" + string(caPEM) + } + configHashKey = crypto.StringToSHA1(hashInput) return buff.String(), configHashKey, nil } // MultiRunnerConfig initialize config for multiple runners object -func MultiRunnerConfig(runnerObject *v1beta2.MultiRunner, tokens map[string]string) (gitlabConfig, configHashKey string, err error) { +func MultiRunnerConfig(runnerObject *v1beta2.MultiRunner, tokens map[string]string, caPEM []byte) (gitlabConfig, configHashKey string, err error) { // create configuration for the runners var runners []*config.RunnerConfig for _, entry := range runnerObject.Spec.Entries { @@ -125,6 +132,13 @@ func MultiRunnerConfig(runnerObject *v1beta2.MultiRunner, tokens map[string]stri } gitlabConfig = buff.String() - configHashKey = crypto.StringToSHA1(gitlabConfig) + // Fold the CA bundle into the hash so a CA content change (rotation) bumps + // the config version and rolls the deployment, even though config.toml only + // references tls-ca-file by path. Empty caPEM keeps the no-CA hash unchanged. + hashInput := gitlabConfig + if len(caPEM) > 0 { + hashInput += "\n" + string(caPEM) + } + configHashKey = crypto.StringToSHA1(hashInput) return buff.String(), configHashKey, nil } diff --git a/internal/generate/config_test.go b/internal/generate/config_test.go index a6ee5fe..ae0c567 100644 --- a/internal/generate/config_test.go +++ b/internal/generate/config_test.go @@ -20,7 +20,7 @@ func TestSingleRunnerConfig_TLSCAFile(t *testing.T) { } tokens := map[string]string{"r1": "glrt-token"} - cfg, _, err := SingleRunnerConfig(base, tokens) + cfg, _, err := SingleRunnerConfig(base, tokens, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -30,7 +30,7 @@ func TestSingleRunnerConfig_TLSCAFile(t *testing.T) { withCA := base.DeepCopy() withCA.Spec.CACertificate = &v1beta2.CASource{ConfigMapKeyRef: &v1beta2.CAKeyRef{Name: "ca-cm"}} - cfg, _, err = SingleRunnerConfig(withCA, tokens) + cfg, _, err = SingleRunnerConfig(withCA, tokens, []byte("ca-pem")) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -41,7 +41,7 @@ func TestSingleRunnerConfig_TLSCAFile(t *testing.T) { // an inline value sets tls-ca-file the same way as a ref inline := base.DeepCopy() inline.Spec.CACertificate = &v1beta2.CASource{Value: "-----BEGIN CERTIFICATE-----\nx\n-----END CERTIFICATE-----\n"} - cfg, _, err = SingleRunnerConfig(inline, tokens) + cfg, _, err = SingleRunnerConfig(inline, tokens, []byte("ca-pem")) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -60,7 +60,7 @@ func TestMultiRunnerConfig_TLSCAFile(t *testing.T) { } tokens := map[string]string{"e1": "glrt-a", "e2": "glrt-b"} - cfg, _, err := MultiRunnerConfig(base, tokens) + cfg, _, err := MultiRunnerConfig(base, tokens, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -70,7 +70,7 @@ func TestMultiRunnerConfig_TLSCAFile(t *testing.T) { withCA := base.DeepCopy() withCA.Spec.CACertificate = &v1beta2.CASource{SecretKeyRef: &v1beta2.CAKeyRef{Name: "ca-secret"}} - cfg, _, err = MultiRunnerConfig(withCA, tokens) + cfg, _, err = MultiRunnerConfig(withCA, tokens, []byte("ca-pem")) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -79,3 +79,36 @@ func TestMultiRunnerConfig_TLSCAFile(t *testing.T) { t.Fatalf("expected tls-ca-file on both entries (2), got %d in:\n%s", got, cfg) } } + +// TestSingleRunnerConfig_CAInHash verifies the CA bytes are folded into the +// config hash, so a CA rotation rolls the deployment even though config.toml +// only references tls-ca-file by path. +func TestSingleRunnerConfig_CAInHash(t *testing.T) { + r := &v1beta2.Runner{ + ObjectMeta: metav1.ObjectMeta{Name: "r1", Namespace: "ns"}, + Spec: v1beta2.RunnerSpec{ + GitlabInstanceURL: "https://gitlab.example.com", + CACertificate: &v1beta2.CASource{Value: "inline"}, + }, + } + tokens := map[string]string{"r1": "glrt-token"} + + _, h1, err := SingleRunnerConfig(r, tokens, []byte("ca-one")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + _, h2, err := SingleRunnerConfig(r, tokens, []byte("ca-two")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if h1 == h2 { + t.Fatal("expected the config hash to change when the CA bundle changes") + } + _, h3, err := SingleRunnerConfig(r, tokens, []byte("ca-one")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if h1 != h3 { + t.Fatal("expected the same CA bundle to produce the same hash") + } +}