From 2fee4e842edd3904949091422a5f30182de8f43e Mon Sep 17 00:00:00 2001 From: samirtahir91 <30797145+samirtahir91@users.noreply.github.com> Date: Sat, 13 Apr 2024 13:49:13 +0100 Subject: [PATCH] fix: refactor code for vault, http and k8s client (#39) --- cmd/main.go | 29 +++++- internal/controller/githubapp_controller.go | 92 +++++++++++++++---- .../controller/githubapp_controller_test.go | 30 ------ internal/controller/suite_test.go | 29 ++++-- internal/controller/vault.go | 82 +++-------------- 5 files changed, 132 insertions(+), 130 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 52bdcf8..0f32207 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -24,7 +24,11 @@ import ( // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. + vault "github.com/hashicorp/vault/api" // vault client + kubernetes "k8s.io/client-go/kubernetes" // k8s client _ "k8s.io/client-go/plugin/pkg/client/auth" + "net/http" // http client + ctrlConfig "sigs.k8s.io/controller-runtime/pkg/client/config" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -101,6 +105,22 @@ func main() { TLSOpts: tlsOpts, }) + // http client + httpClient := &http.Client{} + + // Initialise vault client with VAULT_ADDRESS env var + vaultAddress := os.Getenv("VAULT_ADDRESS") // Vault server fqdn + vaultClient, err := vault.NewClient(&vault.Config{ + Address: vaultAddress, + }) + if err != nil { + setupLog.Error(err, "failed to initialise Vault client") + os.Exit(1) + } + + // Initialise K8s client + k8sClientset := kubernetes.NewForConfigOrDie(ctrlConfig.GetConfigOrDie()) + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme, Metrics: metricsserver.Options{ @@ -130,9 +150,12 @@ func main() { } if err = (&controller.GithubAppReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Recorder: mgr.GetEventRecorderFor("githubapp-controller"), + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Recorder: mgr.GetEventRecorderFor("githubapp-controller"), + HTTPClient: httpClient, + VaultClient: vaultClient, + K8sClient: k8sClientset, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "GithubApp") os.Exit(1) diff --git a/internal/controller/githubapp_controller.go b/internal/controller/githubapp_controller.go index b142efa..ed29cfd 100644 --- a/internal/controller/githubapp_controller.go +++ b/internal/controller/githubapp_controller.go @@ -27,12 +27,14 @@ import ( "time" githubappv1 "github-app-operator/api/v1" + vault "github.com/hashicorp/vault/api" // vault client appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + kubernetes "k8s.io/client-go/kubernetes" // k8s client "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" // Required for Watching @@ -46,9 +48,12 @@ import ( // GithubAppReconciler reconciles a GithubApp object type GithubAppReconciler struct { client.Client - Scheme *runtime.Scheme - Recorder record.EventRecorder - lock sync.Mutex + Scheme *runtime.Scheme + Recorder record.EventRecorder + HTTPClient *http.Client + VaultClient *vault.Client + K8sClient *kubernetes.Clientset + lock sync.Mutex } var ( @@ -56,9 +61,10 @@ var ( defaultTimeBeforeExpiry = 15 * time.Minute // Default time before expiry reconcileInterval time.Duration // Requeue interval (from env var) timeBeforeExpiry time.Duration // Expiry threshold (from env var) - vaultAddress = os.Getenv("VAULT_ADDRESS") // Vault server fqdn vaultAudience = os.Getenv("VAULT_ROLE_AUDIENCE") // Vault audience bound to role vaultRole = os.Getenv("VAULT_ROLE") // Vault role to use + serviceAccountName string // controller service account + kubernetesNamespace string // controller namespace ) const ( @@ -224,7 +230,7 @@ func (r *GithubAppReconciler) checkExpiryAndUpdateAccessToken(ctx context.Contex username := string(accessTokenSecret.Data["username"]) // Check if the access token is a valid github token via gh api auth - if !isAccessTokenValid(ctx, username, accessToken) { + if !r.isAccessTokenValid(ctx, username, accessToken) { // If accessToken is invalid, generate or update access token return r.generateOrUpdateAccessToken(ctx, githubApp) } @@ -245,7 +251,7 @@ func (r *GithubAppReconciler) checkExpiryAndUpdateAccessToken(ctx context.Contex } // Function to check if the access token is valid by making a request to GitHub API -func isAccessTokenValid(ctx context.Context, username string, accessToken string) bool { +func (r *GithubAppReconciler) isAccessTokenValid(ctx context.Context, username string, accessToken string) bool { l := log.FromContext(ctx) // If username has been modified, renew the secret @@ -259,9 +265,6 @@ func isAccessTokenValid(ctx context.Context, username string, accessToken string // GitHub API endpoint for rate limit information url := "https://api.github.com/rate_limit" - // Create a new HTTP client - client := &http.Client{} - // Create a new request ghReq, err := http.NewRequest("GET", url, nil) if err != nil { @@ -273,7 +276,7 @@ func isAccessTokenValid(ctx context.Context, username string, accessToken string ghReq.Header.Set("Authorization", "token "+accessToken) // Send the request - resp, err := client.Do(ghReq) + resp, err := r.HTTPClient.Do(ghReq) if err != nil { l.Error(err, "Error sending request to GitHub API for rate limit") return false @@ -359,16 +362,16 @@ func (r *GithubAppReconciler) getPrivateKeyFromSecret(ctx context.Context, githu } // Function to get private key from a Vault secret -func getPrivateKeyFromVault(ctx context.Context, mountPath string, secretPath string, secretKey string) ([]byte, error) { +func (r *GithubAppReconciler) getPrivateKeyFromVault(ctx context.Context, mountPath string, secretPath string, secretKey string) ([]byte, error) { // Get JWT from k8s Token Request API - token, err := RequestToken(ctx, vaultAudience) + token, err := r.RequestToken(ctx, vaultAudience, kubernetesNamespace, serviceAccountName) if err != nil { return []byte(""), err } // Get private key from Vault secret with short-lived JWT - privateKey, err := GetSecretWithKubernetesAuth(token, vaultAddress, vaultRole, mountPath, secretPath, secretKey) + privateKey, err := r.GetSecretWithKubernetesAuth(token, vaultRole, mountPath, secretPath, secretKey) if err != nil { return []byte(""), err } @@ -386,14 +389,14 @@ func (r *GithubAppReconciler) generateOrUpdateAccessToken(ctx context.Context, g // Vault auth will take precedence over using `spec.privateKeySecret` if githubApp.Spec.VaultPrivateKey != nil { - if vaultAddress == "" || vaultAudience == "" || vaultRole == "" { + if r.VaultClient.Address() == "" || vaultAudience == "" || vaultRole == "" { return fmt.Errorf("failed on vault auth: VAULT_ROLE, VAULT_ROLE_AUDIENCE and VAULT_ADDRESS are required env variables for Vault authentication") } mountPath := githubApp.Spec.VaultPrivateKey.MountPath secretPath := githubApp.Spec.VaultPrivateKey.SecretPath secretKey := githubApp.Spec.VaultPrivateKey.SecretKey - privateKey, privateKeyErr = getPrivateKeyFromVault(ctx, mountPath, secretPath, secretKey) + privateKey, privateKeyErr = r.getPrivateKeyFromVault(ctx, mountPath, secretPath, secretKey) if privateKeyErr != nil { return fmt.Errorf("failed to get private key from vault: %v", privateKeyErr) } @@ -406,7 +409,7 @@ func (r *GithubAppReconciler) generateOrUpdateAccessToken(ctx context.Context, g } // Generate or renew access token - accessToken, expiresAt, err := generateAccessToken( + accessToken, expiresAt, err := r.generateAccessToken( ctx, githubApp.Spec.AppId, githubApp.Spec.InstallId, @@ -556,7 +559,7 @@ func updateGithubAppStatusWithRetry(ctx context.Context, r *GithubAppReconciler, } // function to generate new access token for gh app -func generateAccessToken(ctx context.Context, appID int, installationID int, privateKey []byte) (string, metav1.Time, error) { +func (r *GithubAppReconciler) generateAccessToken(ctx context.Context, appID int, installationID int, privateKey []byte) (string, metav1.Time, error) { l := log.FromContext(ctx) @@ -579,8 +582,7 @@ func generateAccessToken(ctx context.Context, appID int, installationID int, pri return "", metav1.Time{}, fmt.Errorf("failed to sign JWT: %v", err) } - // Create HTTP client and perform request to get installation token - httpClient := &http.Client{} + // Use HTTP client and perform request to get installation token url := fmt.Sprintf("https://api.github.com/app/installations/%d/access_tokens", installationID) req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, url, nil) if err != nil { @@ -590,7 +592,7 @@ func generateAccessToken(ctx context.Context, appID int, installationID int, pri req.Header.Set("Accept", "application/vnd.github+json") // Send post request for access token - resp, err := httpClient.Do(req) + resp, err := r.HTTPClient.Do(req) if err != nil { return "", metav1.Time{}, fmt.Errorf("failed to perform HTTP request: %v", err) } @@ -722,6 +724,48 @@ func githubAppPredicate() predicate.Predicate { } } +// Function to get service account and namespace of controller +func getServiceAccountAndNamespace() (string, string, error) { + + // Get KSA mounted in pod + serviceAccountToken, err := os.ReadFile("var/run/secrets/kubernetes.io/serviceaccount/token") + if err != nil { + return "", "", fmt.Errorf("failed to read service account token: %v", err) + } + // Parse the KSA token + parsedToken, _, err := new(jwt.Parser).ParseUnverified(string(serviceAccountToken), jwt.MapClaims{}) + if err != nil { + return "", "", fmt.Errorf("failed to parse token: %v", err) + } + // Get the claims + claims, ok := parsedToken.Claims.(jwt.MapClaims) + if !ok { + return "", "", fmt.Errorf("failed to parse token claims") + } + // Get kubernetes.io claims + kubernetesClaims, ok := claims["kubernetes.io"].(map[string]interface{}) + if !ok { + return "", "", fmt.Errorf("failed to assert kubernetes.io claim to map[string]interface{}") + } + // Get serviceaccount claim + serviceAccountClaims, ok := kubernetesClaims["serviceaccount"].(map[string]interface{}) + if !ok { + return "", "", fmt.Errorf("failed to assert serviceaccount claim to map[string]interface{}") + } + // Get the namespace + kubernetesNamespace, ok := kubernetesClaims["namespace"].(string) + if !ok { + return "", "", fmt.Errorf("failed to assert namespace to string") + } + // Get service account name + serviceAccountName, ok := serviceAccountClaims["name"].(string) + if !ok { + return "", "", fmt.Errorf("failed to assert service account name to string") + } + + return serviceAccountName, kubernetesNamespace, nil +} + // SetupWithManager sets up the controller with the Manager. func (r *GithubAppReconciler) SetupWithManager(mgr ctrl.Manager) error { // Get reconcile interval from environment variable or use default value @@ -743,6 +787,14 @@ func (r *GithubAppReconciler) SetupWithManager(mgr ctrl.Manager) error { timeBeforeExpiry = defaultTimeBeforeExpiry } + // Get service account name and namespace + serviceAccountName, kubernetesNamespace, err = getServiceAccountAndNamespace() + if err != nil { + log.Log.Error(err, "failed to get service account and/or namespace of controller") + } else { + log.Log.Info("Got controller aervice account and namespace", "service account", serviceAccountName, "namespace", kubernetesNamespace) + } + return ctrl.NewControllerManagedBy(mgr). // Watch GithubApps For(&githubappv1.GithubApp{}, builder.WithPredicates(predicate.ResourceVersionChangedPredicate{}, githubAppPredicate())). diff --git a/internal/controller/githubapp_controller_test.go b/internal/controller/githubapp_controller_test.go index 0d3ad3b..ff4f8a0 100644 --- a/internal/controller/githubapp_controller_test.go +++ b/internal/controller/githubapp_controller_test.go @@ -30,7 +30,6 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/reconcile" ) const ( @@ -147,35 +146,6 @@ var _ = Describe("GithubApp controller", func() { }) }) - Context("When requeing a reconcile for a GithubApp that is not expired", func() { - It("should successfully reconcile the resource and get the rate limit", func() { - ctx := context.Background() - - By("Reconciling the created resource") - controllerReconciler := &GithubAppReconciler{ - Client: k8sClient, - Scheme: k8sClient.Scheme(), - } - - // Perform reconciliation for the resource - result, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ - NamespacedName: types.NamespacedName{ - Namespace: namespace1, - Name: githubAppName, - }, - }) - - // Verify if reconciliation was successful - Expect(err).NotTo(HaveOccurred(), fmt.Sprintf("Reconciliation failed: %v", err)) - - // Print the result - fmt.Println("Reconciliation result:", result) - - // Delete the GitHubApp after reconciliation - test_helpers.DeleteGitHubAppAndWait(ctx, k8sClient, namespace1, githubAppName) - }) - }) - Context("When reconciling a GithubApp with spec.rolloutDeployment.labels.foo as bar", func() { It("Should eventually upgrade the deployment matching label foo: bar", func() { if os.Getenv("USE_EXISTING_CLUSTER") == "" { diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 40a07eb..ed94bc9 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -19,11 +19,15 @@ package controller import ( "context" "fmt" + "net/http" // http client "os" "path/filepath" "runtime" "testing" + vault "github.com/hashicorp/vault/api" // vault client + kubernetes "k8s.io/client-go/kubernetes" // k8s client + ctrl "sigs.k8s.io/controller-runtime" . "github.com/onsi/ginkgo/v2" @@ -44,11 +48,14 @@ import ( // http://onsi.github.io/ginkgo/ to learn more about Ginkgo. var ( - cfg *rest.Config - k8sClient client.Client - testEnv *envtest.Environment - ctx context.Context - cancel context.CancelFunc + cfg *rest.Config + k8sClient client.Client + httpClient *http.Client + vaultClient *vault.Client + k8sClientset *kubernetes.Clientset + testEnv *envtest.Environment + ctx context.Context + cancel context.CancelFunc ) func TestControllers(t *testing.T) { @@ -104,10 +111,16 @@ var _ = BeforeSuite(func() { }) Expect(err).ToNot(HaveOccurred()) + // http client + httpClient = &http.Client{} + err = (&GithubAppReconciler{ - Client: k8sManager.GetClient(), - Scheme: k8sManager.GetScheme(), - Recorder: k8sManager.GetEventRecorderFor("githubapp-controller"), + Client: k8sManager.GetClient(), + Scheme: k8sManager.GetScheme(), + Recorder: k8sManager.GetEventRecorderFor("githubapp-controller"), + HTTPClient: httpClient, + VaultClient: vaultClient, + K8sClient: k8sClientset, }).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) diff --git a/internal/controller/vault.go b/internal/controller/vault.go index 5eb169e..2e518bf 100644 --- a/internal/controller/vault.go +++ b/internal/controller/vault.go @@ -20,31 +20,20 @@ import ( "context" "encoding/base64" "fmt" - "github.com/golang-jwt/jwt/v4" - "k8s.io/client-go/kubernetes" - "k8s.io/client-go/rest" "k8s.io/utils/ptr" - "os" - // vault auth - vault "github.com/hashicorp/vault/api" - auth "github.com/hashicorp/vault/api/auth/kubernetes" - // k8s Token request - authenticationv1 "k8s.io/api/authentication/v1" + auth "github.com/hashicorp/vault/api/auth/kubernetes" // vault k8s auth + authenticationv1 "k8s.io/api/authentication/v1" // k8s Token request metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // Function to create token via K8s Token Request API -func RequestToken(ctx context.Context, vaultAudience string) (string, error) { - // Auth to k8s using mounted service account - config, err := rest.InClusterConfig() - if err != nil { - return "", fmt.Errorf("failed to use in cluster config: %v", err) - } - clientset, err := kubernetes.NewForConfig(config) - if err != nil { - return "", fmt.Errorf("failed to set k8s clientset: %v", err) - } +func (r *GithubAppReconciler) RequestToken( + ctx context.Context, + vaultAudience string, + kubernetesNamespace string, + serviceAccountName string, +) (string, error) { // Token request spec // TTL of 10 mins for short lived JWT for Vault auth @@ -55,50 +44,14 @@ func RequestToken(ctx context.Context, vaultAudience string) (string, error) { }, } - // Get KSA mounted in pod - serviceAccountToken, err := os.ReadFile("var/run/secrets/kubernetes.io/serviceaccount/token") - if err != nil { - return "", fmt.Errorf("failed to read service account token: %v", err) - } - // Parse the KSA token - parsedToken, _, err := new(jwt.Parser).ParseUnverified(string(serviceAccountToken), jwt.MapClaims{}) - if err != nil { - return "", fmt.Errorf("failed to parse token: %v", err) - } - // Get the claims - claims, ok := parsedToken.Claims.(jwt.MapClaims) - if !ok { - return "", fmt.Errorf("failed to parse token claims") - } - // Get kubernetes.io claims - kubernetesClaims, ok := claims["kubernetes.io"].(map[string]interface{}) - if !ok { - return "", fmt.Errorf("failed to assert kubernetes.io claim to map[string]interface{}") - } - // Get serviceaccount claim - serviceAccountClaims, ok := kubernetesClaims["serviceaccount"].(map[string]interface{}) - if !ok { - return "", fmt.Errorf("failed to assert serviceaccount claim to map[string]interface{}") - } - // Get the namespace - kubernetesNamespace, ok := kubernetesClaims["namespace"].(string) - if !ok { - return "", fmt.Errorf("failed to assert namespace to string") - } - // Get service account name - serviceAccountName, ok := serviceAccountClaims["name"].(string) - if !ok { - return "", fmt.Errorf("failed to assert service account name to string") - } - // Request a JWT from Token Request API - tokenRequest, tErr := clientset.CoreV1().ServiceAccounts(kubernetesNamespace).CreateToken( + tokenRequest, err := r.K8sClient.CoreV1().ServiceAccounts(kubernetesNamespace).CreateToken( ctx, serviceAccountName, treq, metav1.CreateOptions{}, ) - if tErr != nil { + if err != nil { return "", fmt.Errorf("failed to create token request to k8s api: %v", err) } token := tokenRequest.Status.Token @@ -106,23 +59,14 @@ func RequestToken(ctx context.Context, vaultAudience string) (string, error) { } // Fetches a key-value secret (kv-2) after authenticating to Vault with a Kubernetes service account -func GetSecretWithKubernetesAuth( +func (r *GithubAppReconciler) GetSecretWithKubernetesAuth( token string, - vaultAddress string, vaultRole string, mountPath string, secretPath string, secretKey string, ) ([]byte, error) { - // Initialise vault client - client, err := vault.NewClient(&vault.Config{ - Address: vaultAddress, - }) - if err != nil { - return []byte(""), fmt.Errorf("failed to initialise Vault client: %v", err) - } - // Auth to Vault using k8s auth, role and short-lived JWT with defined audience k8sAuth, err := auth.NewKubernetesAuth( vaultRole, @@ -131,7 +75,7 @@ func GetSecretWithKubernetesAuth( if err != nil { return []byte(""), fmt.Errorf("failed auth to vault using k8s auth with JWT: %v", err) } - authInfo, err := client.Auth().Login(context.Background(), k8sAuth) + authInfo, err := r.VaultClient.Auth().Login(context.Background(), k8sAuth) if err != nil { return []byte(""), fmt.Errorf("failed to login to vault with k8s auth: %v", err) } @@ -140,7 +84,7 @@ func GetSecretWithKubernetesAuth( } // Get secret from vault mount path - secret, err := client.KVv2(mountPath).Get(context.Background(), secretPath) + secret, err := r.VaultClient.KVv2(mountPath).Get(context.Background(), secretPath) if err != nil { return []byte(""), fmt.Errorf("failed to read secret in vault: %v", err) }