Skip to content

Commit

Permalink
Fix wrong ca bundle getter
Browse files Browse the repository at this point in the history 
Signed-off-by: Damien Dassieu <dassieu.damien@gmail.com>
  • Loading branch information
@damsien
damsien committed Jan 10, 2025
1 parent 0aaebaa commit 4fe0ded
Show file tree
Hide file tree
Showing 24 changed files with 688 additions and 52 deletions.
1 change: 1 addition & 0 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ setup-gitea: ## Setup the 2 gitea platforms in the cluster

.PHONY: cleanup-gitea
cleanup-gitea: ## Cleanup the 2 gitea platforms from the cluster.
rm -rf /tmp/gitea-certs
helm uninstall gitea -n jupyter
kubectl delete ns jupyter
helm uninstall gitea -n saturn
Expand Down
10 changes: 5 additions & 5 deletions internal/interceptor/git_pusher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type GitPusher struct {
gitToken string
operation admissionv1.Operation
insecureSkipTlsVerify bool
caBundle string
caBundle []byte
}

type GitPushResponse struct {
Expand All @@ -58,8 +58,8 @@ func (gp *GitPusher) Push() (GitPushResponse, error) {
InsecureSkipTLS: gp.insecureSkipTlsVerify,
Progress: io.MultiWriter(&verboseOutput),
}
if gp.caBundle != "" {
cloneOption.CABundle = []byte(gp.caBundle)
if gp.caBundle != nil {
cloneOption.CABundle = gp.caBundle
}
repo, err := git.Clone(memory.NewStorage(), memfs.New(), cloneOption)
if err != nil {
Expand Down Expand Up @@ -268,8 +268,8 @@ func (gp *GitPusher) pushChanges(repo *git.Repository) error {
InsecureSkipTLS: gp.insecureSkipTlsVerify,
Progress: io.MultiWriter(&verboseOutput), // Capture verbose output
}
if gp.caBundle != "" {
pushOptions.CABundle = []byte(gp.caBundle)
if gp.caBundle != nil {
pushOptions.CABundle = gp.caBundle
}
err := repo.Push(pushOptions)
if err != nil {
Expand Down
14 changes: 10 additions & 4 deletions internal/interceptor/webhook_request_checker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ type wrcDetails struct {
commitHash string
gitUser gitUser
insecureSkipTlsVerify bool
caBundle string
caBundle []byte
pushDetails string

// Error
Expand Down Expand Up @@ -419,18 +419,24 @@ func (wrc *WebhookRequestChecker) convertToYaml(details *wrcDetails) error {

func (wrc *WebhookRequestChecker) tlsContructor(details *wrcDetails) error {
// Step 1: Search for the global CA Bundle of the server located in the syngit namespace
caBundle, caErr := utils.FindGlobalCABundle(wrc.k8sClient, details.repoHost)
caBundle, caErr := utils.FindGlobalCABundle(wrc.k8sClient, strings.Split(details.repoHost, ":")[0])
if caErr != nil && strings.Contains(caErr.Error(), utils.CaSecretWrongTypeErrorMessage) {
details.messageAddition = caErr.Error()
return caErr
}

// Step 2: Search for a specific CA Bundle located in the current namespace
caBundleSecretRef := wrc.remoteSyncer.Spec.CABundleSecretRef
caBundleRsy, caErr := utils.FindCABundle(wrc.k8sClient, caBundleSecretRef.Namespace, caBundleSecretRef.Name)
ns := caBundleSecretRef.Namespace
if ns == "" {
ns = wrc.remoteSyncer.Namespace
}
caBundleRsy, caErr := utils.FindCABundle(wrc.k8sClient, ns, caBundleSecretRef.Name)
if caErr != nil {
details.messageAddition = caErr.Error()
return caErr
}
if caBundleRsy != "" {
if caBundleRsy != nil {
caBundle = caBundleRsy
}

Expand Down
14 changes: 7 additions & 7 deletions pkg/utils/ca-finder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ import (

const CaSecretWrongTypeErrorMessage = "the CA bundle secret must be of type \"kubernetes.io/ts\""

func FindGlobalCABundle(client client.Client, host string) (string, error) {
return FindCABundle(client, os.Getenv("MANAGER_NAMESPACE"), host+"-ca-bundle")
func FindGlobalCABundle(client client.Client, host string) ([]byte, error) {
return FindCABundle(client, os.Getenv("MANAGER_NAMESPACE"), host+"-ca-cert")
}

func FindCABundle(client client.Client, namespace string, name string) (string, error) {
func FindCABundle(client client.Client, namespace string, name string) ([]byte, error) {
if name == "" {
return "", nil
return nil, nil
}

ctx := context.Background()
Expand All @@ -27,10 +27,10 @@ func FindCABundle(client client.Client, namespace string, name string) (string,

err := client.Get(ctx, globalNamespacedName, caBundleSecret)
if err != nil {
return "", err
return nil, err
}
if caBundleSecret.Type != "kubernetes.io/tls" {
return "", errors.New(CaSecretWrongTypeErrorMessage)
return nil, errors.New(CaSecretWrongTypeErrorMessage)
}
return caBundleSecret.StringData["tls.crt"], nil
return caBundleSecret.Data["tls.crt"], nil
}
3 changes: 2 additions & 1 deletion test/e2e/syngit/02_commitonly_cm_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,15 @@ var _ = Describe("02 CommitOnly a ConfigMap", func() {
return err == nil
}, timeout, interval).Should(BeTrue())

repoUrl := "http://" + gitP1Fqdn + "/syngituser/blue.git"
repoUrl := "https://" + gitP1Fqdn + "/syngituser/blue.git"
By("creating the RemoteSyncer")
remotesyncer := &syngit.RemoteSyncer{
ObjectMeta: metav1.ObjectMeta{
Name: remoteSyncerName,
Namespace: namespace,
},
Spec: syngit.RemoteSyncerSpec{
InsecureSkipTlsVerify: true,
DefaultBlockAppliedMessage: defaultDeniedMessage,
DefaultBranch: "main",
DefaultUnauthorizedUserMode: syngit.Block,
Expand Down
3 changes: 2 additions & 1 deletion test/e2e/syngit/03_commitapply_cm_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,15 @@ var _ = Describe("03 CommitApply a ConfigMap", func() {
return err == nil
}, timeout, interval).Should(BeTrue())

repoUrl := "http://" + gitP1Fqdn + "/syngituser/blue.git"
repoUrl := "https://" + gitP1Fqdn + "/syngituser/blue.git"
By("creating the RemoteSyncer")
remotesyncer := &syngit.RemoteSyncer{
ObjectMeta: metav1.ObjectMeta{
Name: remoteSyncerName,
Namespace: namespace,
},
Spec: syngit.RemoteSyncerSpec{
InsecureSkipTlsVerify: true,
DefaultBranch: "main",
DefaultUnauthorizedUserMode: syngit.Block,
ExcludedFields: []string{".metadata.uid"},
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/syngit/04_excludedfields_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package e2e_syngit

import (
"context"
"fmt"
"strings"

. "github.com/onsi/ginkgo/v2"
Expand Down Expand Up @@ -70,14 +69,15 @@ var _ = Describe("04 Create RemoteSyncer with excluded fields", func() {
return err == nil
}, timeout, interval).Should(BeTrue())

repoUrl := "http://" + gitP1Fqdn + "/syngituser/blue.git"
repoUrl := "https://" + gitP1Fqdn + "/syngituser/blue.git"
By("creating the RemoteSyncer")
remotesyncer := &syngit.RemoteSyncer{
ObjectMeta: metav1.ObjectMeta{
Name: remoteSyncerName,
Namespace: namespace,
},
Spec: syngit.RemoteSyncerSpec{
InsecureSkipTlsVerify: true,
DefaultBlockAppliedMessage: defaultDeniedMessage,
DefaultBranch: "main",
DefaultUnauthorizedUserMode: syngit.Block,
Expand Down Expand Up @@ -134,7 +134,6 @@ var _ = Describe("04 Create RemoteSyncer with excluded fields", func() {
cm,
metav1.CreateOptions{},
)
fmt.Println(err)
return err != nil && strings.Contains(err.Error(), defaultDeniedMessage)
}, timeout, interval).Should(BeTrue())

Expand Down Expand Up @@ -215,14 +214,15 @@ var _ = Describe("04 Create RemoteSyncer with excluded fields", func() {
)
Expect(err).ToNot(HaveOccurred())

repoUrl := "http://" + gitP1Fqdn + "/syngituser/blue.git"
repoUrl := "https://" + gitP1Fqdn + "/syngituser/blue.git"
By("creating the RemoteSyncer")
remotesyncer := &syngit.RemoteSyncer{
ObjectMeta: metav1.ObjectMeta{
Name: remoteSyncerName,
Namespace: namespace,
},
Spec: syngit.RemoteSyncerSpec{
InsecureSkipTlsVerify: true,
DefaultBlockAppliedMessage: defaultDeniedMessage,
DefaultBranch: "main",
DefaultUnauthorizedUserMode: syngit.Block,
Expand Down
3 changes: 2 additions & 1 deletion test/e2e/syngit/05_defaultuser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,15 @@ var _ = Describe("05 Use a default user", func() {
return err == nil
}, timeout, interval).Should(BeTrue())

repoUrl := "http://" + gitP1Fqdn + "/syngituser/green.git"
repoUrl := "https://" + gitP1Fqdn + "/syngituser/green.git"
By("creating the RemoteSyncer")
remotesyncer := &syngit.RemoteSyncer{
ObjectMeta: metav1.ObjectMeta{
Name: remoteSyncerName,
Namespace: namespace,
},
Spec: syngit.RemoteSyncerSpec{
InsecureSkipTlsVerify: true,
DefaultBranch: "main",
DefaultUnauthorizedUserMode: syngit.UseDefaultUser,
ExcludedFields: []string{".metadata.uid"},
Expand Down
14 changes: 7 additions & 7 deletions test/e2e/syngit/06_objects_lifecycle_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,10 @@ import (
var _ = Describe("06 Test objects lifecycle", func() {

const (
remoteUserLuffyName = "remoteuser-luffy"
remoteUserLuffyJupyterName = "remoteuser-luffy"
remoteUserLuffySaturnName = "remoteuser-luffy-saturn"
remotesyncerValidationWebhookName = "remotesyncer.syngit.io"
remoteSyncerName = "remotesyncer-test6"
remoteUserLuffyName = "remoteuser-luffy"
remoteUserLuffyJupyterName = "remoteuser-luffy"
remoteUserLuffySaturnName = "remoteuser-luffy-saturn"
remoteSyncerName = "remotesyncer-test6"
)

It("should properly manage the RemoteUserBinding associated webhooks", func() {
Expand Down Expand Up @@ -185,14 +184,15 @@ var _ = Describe("06 Test objects lifecycle", func() {
return err == nil
}, timeout, interval).Should(BeTrue())

repoUrl := "http://" + gitP1Fqdn + "/syngituser/blue.git"
repoUrl := "https://" + gitP1Fqdn + "/syngituser/blue.git"
By("creating the RemoteSyncer")
remotesyncer := &syngit.RemoteSyncer{
ObjectMeta: metav1.ObjectMeta{
Name: remoteSyncerName,
Namespace: namespace,
},
Spec: syngit.RemoteSyncerSpec{
InsecureSkipTlsVerify: true,
DefaultBranch: "main",
DefaultUnauthorizedUserMode: syngit.Block,
ExcludedFields: []string{".metadata.uid"},
Expand Down Expand Up @@ -222,7 +222,7 @@ var _ = Describe("06 Test objects lifecycle", func() {
By("checking that the ValidationWebhhok scopes sts")
Wait3()
nnValidation := types.NamespacedName{
Name: remotesyncerValidationWebhookName,
Name: dynamicWebhookName,
}
getValidation := &admissionv1.ValidatingWebhookConfiguration{}

Expand Down
6 changes: 4 additions & 2 deletions test/e2e/syngit/07_bypass_subject_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,15 @@ var _ = Describe("07 Subject bypasses interception", func() {
return err == nil
}, timeout, interval).Should(BeTrue())

repoUrl := "http://" + gitP1Fqdn + "/syngituser/blue.git"
repoUrl := "https://" + gitP1Fqdn + "/syngituser/blue.git"
By("creating the RemoteSyncer")
remotesyncer := &syngit.RemoteSyncer{
ObjectMeta: metav1.ObjectMeta{
Name: remoteSyncer1Name,
Namespace: namespace,
},
Spec: syngit.RemoteSyncerSpec{
InsecureSkipTlsVerify: true,
DefaultBranch: "main",
DefaultUnauthorizedUserMode: syngit.Block,
BypassInterceptionSubjects: []v1.Subject{{
Expand Down Expand Up @@ -184,14 +185,15 @@ var _ = Describe("07 Subject bypasses interception", func() {
return err == nil
}, timeout, interval).Should(BeTrue())

repoUrl := "http://" + gitP1Fqdn + "/syngituser/blue.git"
repoUrl := "https://" + gitP1Fqdn + "/syngituser/blue.git"
By("creating the RemoteSyncer")
remotesyncer := &syngit.RemoteSyncer{
ObjectMeta: metav1.ObjectMeta{
Name: remoteSyncer2Name,
Namespace: namespace,
},
Spec: syngit.RemoteSyncerSpec{
InsecureSkipTlsVerify: true,
DefaultBranch: "main",
DefaultUnauthorizedUserMode: syngit.Block,
BypassInterceptionSubjects: []v1.Subject{{
Expand Down
7 changes: 5 additions & 2 deletions test/e2e/syngit/08_webhook_rbac_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,15 @@ var _ = Describe("08 Webhook rbac checker", func() {
return err == nil
}, timeout, interval).Should(BeTrue())

repoUrl := "http://" + gitP1Fqdn + "/syngituser/blue.git"
repoUrl := "https://" + gitP1Fqdn + "/syngituser/blue.git"
By("creating the RemoteSyncer for ConfigMaps")
remotesyncer := &syngit.RemoteSyncer{
ObjectMeta: metav1.ObjectMeta{
Name: remoteSyncer1Name,
Namespace: namespace,
},
Spec: syngit.RemoteSyncerSpec{
InsecureSkipTlsVerify: true,
DefaultBranch: "main",
DefaultUnauthorizedUserMode: syngit.Block,
ExcludedFields: []string{".metadata.uid"},
Expand Down Expand Up @@ -177,14 +178,15 @@ var _ = Describe("08 Webhook rbac checker", func() {
return err == nil
}, timeout, interval).Should(BeTrue())

repoUrl := "http://" + gitP1Fqdn + "/syngituser/blue.git"
repoUrl := "https://" + gitP1Fqdn + "/syngituser/blue.git"
By("creating a wrong RemoteSyncer for Secrets")
remotesyncer := &syngit.RemoteSyncer{
ObjectMeta: metav1.ObjectMeta{
Name: remoteSyncer2Name,
Namespace: namespace,
},
Spec: syngit.RemoteSyncerSpec{
InsecureSkipTlsVerify: true,
DefaultBranch: "main",
DefaultUnauthorizedUserMode: syngit.Block,
ExcludedFields: []string{".metadata.uid"},
Expand Down Expand Up @@ -219,6 +221,7 @@ var _ = Describe("08 Webhook rbac checker", func() {
Namespace: namespace,
},
Spec: syngit.RemoteSyncerSpec{
InsecureSkipTlsVerify: true,
DefaultBranch: "main",
DefaultUnauthorizedUserMode: syngit.Block,
ExcludedFields: []string{".metadata.uid"},
Expand Down
10 changes: 7 additions & 3 deletions test/e2e/syngit/09_multi_remotesyncer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,15 @@ var _ = Describe("09 Multi RemoteSyncer test", func() {
return err == nil
}, timeout, interval).Should(BeTrue())

blueUrl := "http://" + gitP1Fqdn + "/syngituser/blue.git"
blueUrl := "https://" + gitP1Fqdn + "/syngituser/blue.git"
By("creating the RemoteSyncer for the blue repo")
blueRemotesyncer := &syngit.RemoteSyncer{
ObjectMeta: metav1.ObjectMeta{
Name: remoteSyncer1Name,
Namespace: namespace,
},
Spec: syngit.RemoteSyncerSpec{
InsecureSkipTlsVerify: true,
DefaultBranch: "main",
DefaultUnauthorizedUserMode: syngit.Block,
ExcludedFields: []string{".metadata.uid"},
Expand All @@ -103,14 +104,15 @@ var _ = Describe("09 Multi RemoteSyncer test", func() {
err = sClient.As(Luffy).CreateOrUpdate(blueRemotesyncer)
Expect(err).ToNot(HaveOccurred())

greenUrl := "http://" + gitP1Fqdn + "/syngituser/green.git"
greenUrl := "https://" + gitP1Fqdn + "/syngituser/green.git"
By("creating the RemoteSyncer for the green repo")
greenRemotesyncer := &syngit.RemoteSyncer{
ObjectMeta: metav1.ObjectMeta{
Name: remoteSyncer2Name,
Namespace: namespace,
},
Spec: syngit.RemoteSyncerSpec{
InsecureSkipTlsVerify: true,
DefaultBranch: "main",
DefaultUnauthorizedUserMode: syngit.Block,
ExcludedFields: []string{".metadata.uid"},
Expand Down Expand Up @@ -217,14 +219,15 @@ var _ = Describe("09 Multi RemoteSyncer test", func() {
return err == nil
}, timeout, interval).Should(BeTrue())

blueUrl := "http://" + gitP1Fqdn + "/syngituser/blue.git"
blueUrl := "https://" + gitP1Fqdn + "/syngituser/blue.git"
By("creating the RemoteSyncer for the blue repo")
blueRemotesyncer := &syngit.RemoteSyncer{
ObjectMeta: metav1.ObjectMeta{
Name: remoteSyncer1Name,
Namespace: namespace,
},
Spec: syngit.RemoteSyncerSpec{
InsecureSkipTlsVerify: true,
DefaultBranch: "main",
DefaultUnauthorizedUserMode: syngit.Block,
ExcludedFields: []string{".metadata.uid"},
Expand Down Expand Up @@ -256,6 +259,7 @@ var _ = Describe("09 Multi RemoteSyncer test", func() {
Namespace: namespace,
},
Spec: syngit.RemoteSyncerSpec{
InsecureSkipTlsVerify: true,
DefaultBranch: "main",
DefaultUnauthorizedUserMode: syngit.Block,
ExcludedFields: []string{".metadata.uid"},
Expand Down
Loading

0 comments on commit 4fe0ded

Please sign in to comment.