From a15666e2998fd0f1ad655134f086d465a69c1615 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Fri, 24 May 2024 14:48:14 +0100 Subject: [PATCH 1/4] Add git CA bundle option for git TLS --- .vscode/launch.json | 9 ++++--- pkg/apiserver/apiserver.go | 4 ++- pkg/cache/cache.go | 5 +++- pkg/cache/cache_test.go | 2 +- pkg/cmd/server/start.go | 3 +++ pkg/engine/clone_test.go | 5 ++++ pkg/git/git.go | 16 +++++++++++ pkg/registry/porch/secret.go | 51 ++++++++++++++++++++++++++++++++++-- pkg/repository/repository.go | 1 + 9 files changed, 87 insertions(+), 9 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 6ae3651e..0729b5f5 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -23,13 +23,14 @@ "program": "${workspaceFolder}/cmd/porch/main.go", "args": [ "--secure-port=9443", - "--v=7", + "--v=6", "--standalone-debug-mode", - "--kubeconfig=${workspaceFolder}/deployments/local/kubeconfig", + "--kubeconfig=${userHome}/.kube/kind-porch-test", "--cache-directory=${workspaceFolder}/.cache", - "--function-runner=192.168.8.202:9445" + "--function-runner=172.18.255.202:9445", + "--use-git-cabundle=true" ], - "cwd": "${workspaceFolder}" + "cwd": "${workspaceFolder}", }, { "name": "Launch Func Client", diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 31e54eda..b0492d0c 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -79,6 +79,7 @@ type ExtraConfig struct { FunctionRunnerAddress string DefaultImagePrefix string RepoSyncFrequency time.Duration + UseGitCaBundle bool } // Config defines the config for the apiserver @@ -212,6 +213,7 @@ func (c completedConfig) New() (*PorchServer, error) { resolverChain := []porch.Resolver{ porch.NewBasicAuthResolver(), + porch.NewCaBundleResolver(), porch.NewGcloudWIResolver(coreV1Client, stsClient), } @@ -223,7 +225,7 @@ func (c completedConfig) New() (*PorchServer, error) { watcherMgr := engine.NewWatcherManager() - cache := cache.NewCache(c.ExtraConfig.CacheDirectory, c.ExtraConfig.RepoSyncFrequency, cache.CacheOptions{ + cache := cache.NewCache(c.ExtraConfig.CacheDirectory, c.ExtraConfig.RepoSyncFrequency, c.ExtraConfig.UseGitCaBundle, cache.CacheOptions{ CredentialResolver: credentialResolver, UserInfoProvider: userInfoProvider, MetadataStore: metadataStore, diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index 5be6cb00..98c145aa 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -51,6 +51,7 @@ type Cache struct { metadataStore meta.MetadataStore repoSyncFrequency time.Duration objectNotifier objectNotifier + useGitCaBundle bool } type objectNotifier interface { @@ -64,7 +65,7 @@ type CacheOptions struct { ObjectNotifier objectNotifier } -func NewCache(cacheDir string, repoSyncFrequency time.Duration, opts CacheOptions) *Cache { +func NewCache(cacheDir string, repoSyncFrequency time.Duration, useGitCaBundle bool, opts CacheOptions) *Cache { return &Cache{ repositories: make(map[string]*cachedRepository), cacheDir: cacheDir, @@ -73,6 +74,7 @@ func NewCache(cacheDir string, repoSyncFrequency time.Duration, opts CacheOption metadataStore: opts.MetadataStore, objectNotifier: opts.ObjectNotifier, repoSyncFrequency: repoSyncFrequency, + useGitCaBundle: useGitCaBundle, } } @@ -136,6 +138,7 @@ func (c *Cache) OpenRepository(ctx context.Context, repositorySpec *configapi.Re CredentialResolver: c.credentialResolver, UserInfoProvider: c.userInfoProvider, MainBranchStrategy: mbs, + UseGitCaBundle: c.useGitCaBundle, }); err != nil { return nil, err } else { diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index f3165cc3..57e8eed5 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -135,7 +135,7 @@ func openRepositoryFromArchive(t *testing.T, ctx context.Context, testPath, name repo, address := git.ServeGitRepository(t, tarfile, tempdir) metadataStore := createMetadataStoreFromArchive(t, "", "") - cache := NewCache(t.TempDir(), 60*time.Second, CacheOptions{ + cache := NewCache(t.TempDir(), 60*time.Second, false, CacheOptions{ MetadataStore: metadataStore, ObjectNotifier: &fakecache.ObjectNotifier{}, }) diff --git a/pkg/cmd/server/start.go b/pkg/cmd/server/start.go index 04b1bda2..599195d1 100644 --- a/pkg/cmd/server/start.go +++ b/pkg/cmd/server/start.go @@ -56,6 +56,7 @@ type PorchServerOptions struct { FunctionRunnerAddress string DefaultImagePrefix string RepoSyncFrequency time.Duration + UseGitCaBundle bool SharedInformerFactory informers.SharedInformerFactory StdOut io.Writer @@ -189,6 +190,7 @@ func (o *PorchServerOptions) Config() (*apiserver.Config, error) { RepoSyncFrequency: o.RepoSyncFrequency, FunctionRunnerAddress: o.FunctionRunnerAddress, DefaultImagePrefix: o.DefaultImagePrefix, + UseGitCaBundle: o.UseGitCaBundle, }, } return config, nil @@ -234,5 +236,6 @@ func (o *PorchServerOptions) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&o.FunctionRunnerAddress, "function-runner", "", "Address of the function runner gRPC service.") fs.StringVar(&o.DefaultImagePrefix, "default-image-prefix", "gcr.io/kpt-fn/", "Default prefix for unqualified function names") fs.StringVar(&o.CacheDirectory, "cache-directory", "", "Directory where Porch server stores repository and package caches.") + fs.BoolVar(&o.UseGitCaBundle, "use-git-cabundle", false, "Determine whether to use a user-defined CaBundle for TLS towards git.") fs.DurationVar(&o.RepoSyncFrequency, "repo-sync-frequency", 60*time.Second, "Frequency in seconds at which registered repositories will be synced.") } diff --git a/pkg/engine/clone_test.go b/pkg/engine/clone_test.go index a1371c56..c98f5abe 100644 --- a/pkg/engine/clone_test.go +++ b/pkg/engine/clone_test.go @@ -170,6 +170,11 @@ type credential struct { username, password string } +// ToString implements repository.Credential. +func (c *credential) ToString() string { + panic("unimplemented") +} + func (c *credential) Valid() bool { return true } diff --git a/pkg/git/git.go b/pkg/git/git.go index c5db3458..46350ee2 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -71,6 +71,7 @@ type GitRepositoryOptions struct { CredentialResolver repository.CredentialResolver UserInfoProvider repository.UserInfoProvider MainBranchStrategy MainBranchStrategy + UseGitCaBundle bool } func OpenRepository(ctx context.Context, name, namespace string, spec *configapi.GitRepository, deployment bool, root string, opts GitRepositoryOptions) (GitRepository, error) { @@ -138,6 +139,14 @@ func OpenRepository(ctx context.Context, name, namespace string, spec *configapi deployment: deployment, } + if opts.UseGitCaBundle { + if caBundle, err := opts.CredentialResolver.ResolveCredential(ctx, namespace, namespace + "-ca-bundle"); err != nil { + klog.Errorf("failed to obtain caBundle from secret %s/%s: %v", namespace, namespace + "-ca-bundle", err) + } else { + repository.caBundle = []byte(caBundle.ToString()) + } + } + if err := repository.fetchRemoteRepository(ctx); err != nil { return nil, err } @@ -178,6 +187,9 @@ type gitRepository struct { deletionProposedCache map[BranchName]bool mutex sync.Mutex + + // caBundle to use for TLS communication towards git + caBundle []byte } var _ GitRepository = &gitRepository{} @@ -884,6 +896,7 @@ func (r *gitRepository) fetchRemoteRepository(ctx context.Context) error { RemoteName: OriginName, Auth: auth, Prune: true, + CABundle: r.caBundle, }) }); err { case nil: // OK @@ -1007,6 +1020,7 @@ func (r *gitRepository) createPackageDeleteCommit(ctx context.Context, branch pl RefSpecs: []config.RefSpec{config.RefSpec(fmt.Sprintf("+%s:%s", local, branch))}, Auth: auth, Tags: git.NoTags, + CABundle: r.caBundle, }) }); err { case nil, git.NoErrAlreadyUpToDate: @@ -1082,6 +1096,7 @@ func (r *gitRepository) pushAndCleanup(ctx context.Context, ph *pushRefSpecBuild RequireRemoteRefs: require, // TODO(justinsb): Need to ensure this is a compare-and-swap Force: true, + CABundle: r.caBundle, }) }); err != nil { return err @@ -1609,6 +1624,7 @@ func (r *gitRepository) commitPackageToMain(ctx context.Context, d *gitPackageDr RemoteName: OriginName, RefSpecs: []config.RefSpec{branch.ForceFetchSpec()}, Auth: auth, + CABundle: r.caBundle, }) }); err { case nil, git.NoErrAlreadyUpToDate: diff --git a/pkg/registry/porch/secret.go b/pkg/registry/porch/secret.go index 98f5f153..f17488d6 100644 --- a/pkg/registry/porch/secret.go +++ b/pkg/registry/porch/secret.go @@ -34,12 +34,15 @@ import ( ) const ( - // Values for scret types supported by porch. + // Values for secret types supported by porch. BasicAuthType = core.SecretTypeBasicAuth WorkloadIdentityAuthType = "kpt.dev/workload-identity-auth" // Annotation used to specify the gsa for a ksa. - WIGCPSAAnnotation = "iam.gke.io/gcp-service-account" + WIGCPSAAnnotation = "iam.gke.io/gcp-service-account" + + //Secret.Data key required for the caBundle + CaBundleDataName = "cabundle" ) func NewCredentialResolver(coreClient client.Reader, resolverChain []Resolver) repository.CredentialResolver { @@ -123,6 +126,10 @@ type BasicAuthCredential struct { Password string } +func (b *BasicAuthCredential) ToString() string { + panic("unimplemented") +} + var _ repository.Credential = &BasicAuthCredential{} func (b *BasicAuthCredential) Valid() bool { @@ -136,6 +143,42 @@ func (b *BasicAuthCredential) ToAuthMethod() transport.AuthMethod { } } +func NewCaBundleResolver() Resolver { + return &CaBundleResolver{} +} + +var _ Resolver = &CaBundleResolver{} + +type CaBundleResolver struct{} + +func (c *CaBundleResolver) Resolve(_ context.Context, secret core.Secret) (repository.Credential, bool, error) { + if secret.Data[CaBundleDataName] == nil { + return nil, false, fmt.Errorf("CaBundle secret.Data key must be set as %s", CaBundleDataName) + } + + return &CaBundleCredential{ + CaBundle: string(secret.Data[CaBundleDataName]), + }, true, nil +} + +type CaBundleCredential struct { + CaBundle string +} + +func (c *CaBundleCredential) ToString() string { + return c.CaBundle +} + +var _ repository.Credential = &CaBundleCredential{} + +func (c *CaBundleCredential) Valid() bool { + return true +} + +func (c *CaBundleCredential) ToAuthMethod() transport.AuthMethod { + panic("unimplemented") +} + func NewGcloudWIResolver(corev1Client *corev1client.CoreV1Client, stsClient *stsv1.Service) Resolver { return &GcloudWIResolver{ coreV1Client: corev1Client, @@ -213,6 +256,10 @@ type GcloudWICredential struct { token *oauth2.Token } +func (b *GcloudWICredential) ToString() string { + panic("unimplemented") +} + var _ repository.Credential = &GcloudWICredential{} func (b *GcloudWICredential) Valid() bool { diff --git a/pkg/repository/repository.go b/pkg/repository/repository.go index d278e9d9..f6a7076b 100644 --- a/pkg/repository/repository.go +++ b/pkg/repository/repository.go @@ -225,6 +225,7 @@ type FunctionRepository interface { type Credential interface { Valid() bool ToAuthMethod() transport.AuthMethod + ToString() string } type CredentialResolver interface { From cfd8e21f4bcbda4d4ebccff72a9d18e1c9e2c7b8 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Mon, 27 May 2024 10:47:54 +0100 Subject: [PATCH 2/4] Update secret key to use ca.crt --- pkg/registry/porch/secret.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/registry/porch/secret.go b/pkg/registry/porch/secret.go index f17488d6..066f708d 100644 --- a/pkg/registry/porch/secret.go +++ b/pkg/registry/porch/secret.go @@ -42,7 +42,7 @@ const ( WIGCPSAAnnotation = "iam.gke.io/gcp-service-account" //Secret.Data key required for the caBundle - CaBundleDataName = "cabundle" + CaBundleDataName = "ca.crt" ) func NewCredentialResolver(coreClient client.Reader, resolverChain []Resolver) repository.CredentialResolver { From a13f48aa009a09d690c663ae3cb9f1f7a3644c02 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Thu, 30 May 2024 15:04:20 +0100 Subject: [PATCH 3/4] Add unit test coverage --- pkg/cache/cache_test.go | 3 +- pkg/cache/fake/credentialresolver.go | 47 ++++++++++++++++++ pkg/registry/porch/secret_test.go | 74 ++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 pkg/cache/fake/credentialresolver.go diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index 57e8eed5..488285c4 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -135,9 +135,10 @@ func openRepositoryFromArchive(t *testing.T, ctx context.Context, testPath, name repo, address := git.ServeGitRepository(t, tarfile, tempdir) metadataStore := createMetadataStoreFromArchive(t, "", "") - cache := NewCache(t.TempDir(), 60*time.Second, false, CacheOptions{ + cache := NewCache(t.TempDir(), 60*time.Second, true, CacheOptions{ MetadataStore: metadataStore, ObjectNotifier: &fakecache.ObjectNotifier{}, + CredentialResolver: &fakecache.CredentialResolver{}, }) cachedGit, err := cache.OpenRepository(ctx, &v1alpha1.Repository{ TypeMeta: metav1.TypeMeta{ diff --git a/pkg/cache/fake/credentialresolver.go b/pkg/cache/fake/credentialresolver.go new file mode 100644 index 00000000..1eff82c4 --- /dev/null +++ b/pkg/cache/fake/credentialresolver.go @@ -0,0 +1,47 @@ +// Copyright 2024 The kpt and Nephio Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package fake + +import ( + "context" + "github.com/nephio-project/porch/pkg/repository" + "github.com/go-git/go-git/v5/plumbing/transport" +) + +type credential struct { + cabundle string +} + +func (c *credential) ToString() string { + return c.cabundle +} + +func (c *credential) Valid() bool { + return true +} + +func (c *credential) ToAuthMethod() transport.AuthMethod { + panic("unimplemented") +} + +type CredentialResolver struct{ + cabundle string +} + +func (cr *CredentialResolver) ResolveCredential(ctx context.Context, namespace, name string) (repository.Credential, error) { + return &credential{ + cabundle: cr.cabundle, + }, nil +} diff --git a/pkg/registry/porch/secret_test.go b/pkg/registry/porch/secret_test.go index 621c173e..022c118b 100644 --- a/pkg/registry/porch/secret_test.go +++ b/pkg/registry/porch/secret_test.go @@ -109,6 +109,80 @@ func TestCredentialResolver(t *testing.T) { } } +func TestCaBundleCredentialResolver(t *testing.T) { + + testCases := map[string]struct { + readerSecret *core.Secret + readerErr error + + resolverCredential repository.Credential + resolverResolved bool + resolverErr error + + expectedCredential repository.Credential + expectedErrString string + }{ + "secret has valid Data key": { + readerSecret: &core.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: secretNamespace, + }, + Type: core.SecretTypeOpaque, + Data: map[string][]byte{ + "ca.crt": []byte("blah"), + }, + }, + expectedCredential: &CaBundleCredential{ + CaBundle: "blah", + }, + }, + "secret has invalid Data key": { + readerSecret: &core.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: secretNamespace, + }, + Type: core.SecretTypeOpaque, + Data: map[string][]byte{ + "invalid": []byte("blah"), + }, + }, + expectedCredential: nil, + expectedErrString: "error resolving credential: CaBundle secret.Data key must be set as ca.crt", + }, + } + + for tn := range testCases { + tc := testCases[tn] + t.Run(tn, func(t *testing.T) { + reader := &fakeReader{ + expectedSecret: tc.readerSecret, + expectedErr: tc.readerErr, + } + credResolver := NewCredentialResolver(reader, []Resolver{ + NewCaBundleResolver(), + &fakeResolver{ + credential: tc.resolverCredential, + resolved: tc.resolverResolved, + err: tc.resolverErr, + }, + }) + + cred, err := credResolver.ResolveCredential(context.Background(), secretNamespace, secretName) + if err != nil { + assert.EqualErrorf(t, err, tc.expectedErrString, "Error should be: %v, got: %v", tc.expectedErrString, err) + } + assert.Equal(t, tc.expectedCredential, cred) + if cred != nil { + assert.Equal(t, cred.ToString(), "blah") + assert.Equal(t, cred.Valid(), true) + assert.Panics(t, func() {cred.ToAuthMethod()}) + } + }) + } +} + type fakeReader struct { expectedSecret *core.Secret expectedErr error From 327ab118faf4abc2661103b6e73be1d44863fe97 Mon Sep 17 00:00:00 2001 From: Fiachra Corcoran Date: Fri, 31 May 2024 13:00:15 +0100 Subject: [PATCH 4/4] Add prelim docuentation for git cabundle --- docs/adding-external-git-ca-bundle.md | 32 +++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 docs/adding-external-git-ca-bundle.md diff --git a/docs/adding-external-git-ca-bundle.md b/docs/adding-external-git-ca-bundle.md new file mode 100644 index 00000000..cba9e0fe --- /dev/null +++ b/docs/adding-external-git-ca-bundle.md @@ -0,0 +1,32 @@ +# Adding an external Git CaBundle + +To enable the porch server to communicate with a custom git deployment over HTTPS, we must: +1. Provide a additional args flag `use-git-cabundle=true` to the porch-server deployment. +2. Provide an additional kubernetes secret containing the relevant certificate chain in the form of a cabundle. + +The secret itself must meet the following criteria: + +- exist in the same `namespace` as the Repository CR (Custom Resource) that requires it +- be named specifically `-ca-bundle` +- have a Data key named `ca.crt` containing the relevant ca certificate (chain) + +For example, a Git Repository is hosted over HTTPS at the following URL: + +`https://my-gitlab.com/joe.bloggs/blueprints.git` + +Before creating the new Repository in the **gitlab** namespace, we must create a secret that fulfils the criteria above. + +`kubectl create secret generic gitlab-ca-bundle --namespace=gitlab --from-file=ca.crt` + +Which would produce the following: + +``` +apiVersion: v1 +kind: Secret +metadata: + name: gitlab-ca-bundle + namespace: gitlab +type: Opaque +data: + ca.crt: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUNuakNDQWdHZ0F3SUJBZ0lRTEdmUytUK3YyRDZDczh1MVBlUUlKREFLQmdncWhrak9QUVFEQkRBZE1Sc3cKR1FZRFZRUURFeEpqWlhKMExXMWhibUZuWlhJdWJHOWpZV3d3SGhjTk1qUXdOVE14TVRFeU5qTXlXaGNOTWpRdwpPREk1TVRFeU5qTXlXakFWTVJNd0VRWURWUVFGRXdveE1qTTBOVFkzT0Rrd01JSUJJakFOQmdrcWhraUc5dzBCCkFRRUZBQU9DQVE4QU1JSUJDZ0tDQVFFQXhCUUtWMEVzQ1JOOGxuV3lQR1ZWNXJwam5QZkI2emszK0N4cEp2NVMKUWhpMG1KbDI0elV1WWZjRzNxdFUva1NuREdjK3NQRUY0RmlOcUlsSTByWHBQSXBPazhKbjEvZU1VT3RkZUUyNgpSWEZBWktjeDVvdUJyZVNja3hsN2RPVkJnOE1EM1h5RU1PQU5nM0hJZ1J4ZWx2U2p1dy8vMURhSlRnK0lBS0dUCkgrOVlRVFcrZDIwSk5wQlR3NkdnQlRsYmdqL2FMRWEwOXVYSVBjK0JUSkpXRThIeDhkVjFNbEtHRFlDU29qZFgKbG9TN1FIa0dsSVk3M0NPZVVGWEVnTlFVVmZaZHdreXNsT3F4WmdXUTNZTFZHcEFyRitjOVdyUGpQQU5NQWtORQpPdHRvaG8zTlRxQ3FST3JEa0RMYWdsU1BKSUd1K25TcU5veVVxSUlWWkV5R1dRSURBUUFCbzJBd1hqQU9CZ05WCkhROEJBZjhFQkFNQ0JhQXdEQVlEVlIwVEFRSC9CQUl3QURBZkJnTlZIU01FR0RBV2dCUitFZTVDTnVJSkcwZjkKV3J3VzdqYUZFeVdzb1RBZEJnTlZIUkVFRmpBVWdoSm5hWFJzWVdJdVpYaGhiWEJzWlM1amIyMHdDZ1lJS29aSQp6ajBFQXdRRGdZb0FNSUdHQWtGLzRyNUM4bnkwdGVIMVJlRzdDdXJHYk02SzMzdTFDZ29GTkthajIva2ovYzlhCnZwODY0eFJKM2ZVSXZGMEtzL1dNUHNad2w2bjMxUWtXT2VpM01aYWtBUUpCREw0Kyt4UUxkMS9uVWdqOW1zN2MKUUx3NXVEMGxqU0xrUS9mOTJGYy91WHc4QWVDck5XcVRqcDEycDJ6MkUzOXRyWWc1a2UvY2VTaWFPUm16eUJuTwpTUTg9Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0= +``` \ No newline at end of file