Skip to content

Commit

Permalink
feat: Add local file cache for private keys (#53)
Browse files Browse the repository at this point in the history
- Private keys are cached in /var/run/github-app-secrets/<namespace>/<githubapp name>
- A githubapp's cached key is wiped on delete of a github app
- Cache key is wiped if there is an error in the access token request from github api for a githubapp
- Reduces calls to kubernetes api for token request and secrets and/or vault
- Up manager seccompProfile for RuntimeDefault
  • Loading branch information
samirtahir91 committed Apr 21, 2024
1 parent 95bfeb9 commit ce3146c
Show file tree
Hide file tree
Showing 8 changed files with 136 additions and 25 deletions.
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Build the manager binary
FROM golang:1.21 AS builder
FROM golang:1.21.9 AS builder
ARG TARGETOS
ARG TARGETARCH

Expand Down
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ EOF
## Getting Started

### Prerequisites
- go version v1.22.1+
- go version v1.21.9+
- docker version 17.03+.
- kubectl version v1.11.3+.
- Access to a Kubernetes v1.11.3+ cluster.
Expand Down Expand Up @@ -195,6 +195,10 @@ Current integration tests cover the scenarios:

**Run the controller in the foreground for testing:**
```sh
# PRIVATE_KEY_CACHE_PATH folder to temp store private keys in local file system
# /tmp/github-test is fine for testing
export PRIVATE_KEY_CACHE_PATH=/tmp/github-test/
# run
make run
```

Expand Down
10 changes: 9 additions & 1 deletion cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,22 @@ func main() {
os.Exit(1)
}

// Path to store private keys for local caching
privateKeyCachePath := "/var/run/github-app-secrets/"
// Check for PRIVATE_KEY_CACHE_PATH environment variable amnd override privateKeyCachePath
if customCachePath := os.Getenv("PRIVATE_KEY_CACHE_PATH"); customCachePath != "" {
// If the environment variable is set, use its value as the privateKeyCachePath
privateKeyCachePath = customCachePath
}

if err = (&controller.GithubAppReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Recorder: mgr.GetEventRecorderFor("githubapp-controller"),
HTTPClient: httpClient,
VaultClient: vaultClient,
K8sClient: k8sClientset,
}).SetupWithManager(mgr); err != nil {
}).SetupWithManager(mgr, privateKeyCachePath); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "GithubApp")
os.Exit(1)
}
Expand Down
6 changes: 6 additions & 0 deletions config/manager/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -1,2 +1,8 @@
resources:
- manager.yaml
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
images:
- name: controller
newName: controller
newTag: latest
20 changes: 14 additions & 6 deletions config/manager/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,15 @@ spec:
# values:
# - linux
securityContext:
# Dockerfile user is 65532
runAsNonRoot: true
# TODO(user): For common cases that do not require escalating privileges
# it is recommended to ensure that all your Pods/Containers are restrictive.
# More info: https://kubernetes.io/docs/concepts/security/pod-security-standards/#restricted
# Please uncomment the following code if your project does NOT have to work on old Kubernetes
runAsUser: 65532
runAsGroup: 65532
fsGroup: 65532
# Please comment the following code if your project DOES have to work on old Kubernetes
# versions < 1.19 or on vendors versions which do NOT support this field by default (i.e. Openshift < 4.11 ).
# seccompProfile:
# type: RuntimeDefault
seccompProfile:
type: RuntimeDefault
containers:
- command:
- /manager
Expand Down Expand Up @@ -112,5 +113,12 @@ spec:
value: githubapp
- name: VAULT_ADDRESS
value: "http://vault.default:8200"
# volume to cache private keys
volumeMounts:
- name: github-app-secrets
mountPath: /var/run/github-app-secrets
volumes:
- name: github-app-secrets
emptyDir: {}
serviceAccountName: controller-manager
terminationGracePeriodSeconds: 10
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github-app-operator

go 1.21
go 1.21.9

require (
github.com/golang-jwt/jwt/v4 v4.5.0
Expand Down
102 changes: 89 additions & 13 deletions internal/controller/githubapp_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"math/rand"
"net/http"
"os"
"path/filepath"
"strconv"
"sync"
"time"
Expand Down Expand Up @@ -87,6 +88,7 @@ var (
vaultRole = os.Getenv("VAULT_ROLE") // Vault role to use
serviceAccountName string // controller service account
kubernetesNamespace string // controller namespace
privateKeyCachePath string // Path to store private keys
)

const (
Expand Down Expand Up @@ -117,11 +119,15 @@ func (r *GithubAppReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
err := r.Get(ctx, req.NamespacedName, githubApp)
if err != nil {
if apierrors.IsNotFound(err) {
l.Info("GithubApp resource not found. Ignoring since object must be deleted.")
l.Info("GithubApp resource not found. Deleting managed objects and .")
// Delete owned access token secret
if err := r.deleteOwnedSecrets(ctx, githubApp); err != nil {
return ctrl.Result{}, err
}
// Delete private key cache
if err := deletePrivateKyCache(req.Namespace, req.Name); err != nil {
return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}
l.Error(err, "failed to get GithubApp")
Expand All @@ -134,11 +140,15 @@ func (r *GithubAppReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
we manually delete the secret.
*/
if !githubApp.ObjectMeta.DeletionTimestamp.IsZero() {
l.Info("GithubApp is being deleted. Deleting managed objects.")
l.Info("GithubApp is being deleted. Deleting managed objects and cache.")
// Delete owned access token secret
if err := r.deleteOwnedSecrets(ctx, githubApp); err != nil {
return ctrl.Result{}, err
}
// Delete private key cache
if err := deletePrivateKyCache(req.Namespace, req.Name); err != nil {
return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}

Expand Down Expand Up @@ -201,6 +211,17 @@ func (r *GithubAppReconciler) deleteOwnedSecrets(ctx context.Context, githubApp
return nil
}

// Function to delete private key cache file for a GithubApp
func deletePrivateKyCache(namespace string, name string) error {
privateKeyPath := filepath.Join(privateKeyCachePath, namespace, name)
// Remove cached private key
err := os.Remove(privateKeyPath)
if err != nil && !os.IsNotExist(err) {
return fmt.Errorf("failed to remove cached private key: %v", err)
}
return nil
}

// Function to update the status field 'Error' of a GithubApp with an error message
func (r *GithubAppReconciler) updateStatusWithError(ctx context.Context, githubApp *githubappv1.GithubApp, errMsg string) error {
// Update the error message in the status field
Expand All @@ -222,7 +243,7 @@ func (r *GithubAppReconciler) checkExpiryAndUpdateAccessToken(ctx context.Contex

// If expiresAt status field is not present or expiry time has already passed, generate or renew access token
if expiresAt.IsZero() || expiresAt.Before(time.Now()) {
return r.generateOrUpdateAccessToken(ctx, githubApp)
return r.createOrUpdateAccessToken(ctx, githubApp)
}

// Check if the access token secret exists if not reconcile immediately
Expand All @@ -234,7 +255,7 @@ func (r *GithubAppReconciler) checkExpiryAndUpdateAccessToken(ctx context.Contex
if err := r.Get(ctx, accessTokenSecretKey, accessTokenSecret); err != nil {
if apierrors.IsNotFound(err) {
// Secret doesn't exist, reconcile straight away
return r.generateOrUpdateAccessToken(ctx, githubApp)
return r.createOrUpdateAccessToken(ctx, githubApp)
}
// Error other than NotFound, return error
return err
Expand All @@ -243,7 +264,7 @@ func (r *GithubAppReconciler) checkExpiryAndUpdateAccessToken(ctx context.Contex
for key := range accessTokenSecret.Data {
if key != "token" && key != "username" {
l.Info("Removing invalid key in access token secret", "Key", key)
return r.generateOrUpdateAccessToken(ctx, githubApp)
return r.createOrUpdateAccessToken(ctx, githubApp)
}
}

Expand All @@ -254,7 +275,7 @@ func (r *GithubAppReconciler) checkExpiryAndUpdateAccessToken(ctx context.Contex
// Check if the access token is a valid github token via gh api auth
if !r.isAccessTokenValid(ctx, username, accessToken) {
// If accessToken is invalid, generate or update access token
return r.generateOrUpdateAccessToken(ctx, githubApp)
return r.createOrUpdateAccessToken(ctx, githubApp)
}

// Access token exists, calculate the duration until expiry
Expand All @@ -265,8 +286,7 @@ func (r *GithubAppReconciler) checkExpiryAndUpdateAccessToken(ctx context.Contex
l.Info(
"Expiry threshold reached - renewing",
)
err := r.generateOrUpdateAccessToken(ctx, githubApp)
return err
return r.createOrUpdateAccessToken(ctx, githubApp)
}

return nil
Expand Down Expand Up @@ -431,16 +451,50 @@ func (r *GithubAppReconciler) getPrivateKeyFromVault(ctx context.Context, mountP
return privateKey, nil
}

// Function to get private key from local file cache
func getPrivateKeyFromCache(namespace string, name string) ([]byte, string, error) {

// Try to get private key from local file system
// Stores keys in <privateKeyCachePath>/<Namespace of githubapp>/<Name of githubapp>
privateKeyDir := filepath.Join(privateKeyCachePath, namespace)
privateKeyPath := filepath.Join(privateKeyDir, name)

// Create dir if does not exist
if _, err := os.Stat(privateKeyDir); os.IsNotExist(err) {
if err := os.MkdirAll(privateKeyDir, 0700); err != nil {
return []byte(""), "", fmt.Errorf("failed to create private key directory: %v", err)
}
}
if _, err := os.Stat(privateKeyPath); err == nil {
// get private key if secret file exists
privateKey, privateKeyErr := os.ReadFile(privateKeyPath)
if privateKeyErr != nil {
return []byte(""), "", fmt.Errorf("failed to read private key from file: %v", privateKeyErr)
}
return privateKey, privateKeyPath, nil
}
// Return privateKeyPath if private key file doesn't exist
return []byte(""), privateKeyPath, nil
}

// Function to generate or update access token
func (r *GithubAppReconciler) generateOrUpdateAccessToken(ctx context.Context, githubApp *githubappv1.GithubApp) error {
func (r *GithubAppReconciler) createOrUpdateAccessToken(ctx context.Context, githubApp *githubappv1.GithubApp) error {
l := log.FromContext(ctx)

var privateKey []byte
var privateKeyErr error
var privateKeyPath string

// Try to get private key from local file system
privateKey, privateKeyPath, privateKeyErr = getPrivateKeyFromCache(githubApp.Namespace, githubApp.Name)
if privateKeyErr != nil {
return fmt.Errorf("failed to read private key from file cache: %v", privateKeyErr)
}

// If private key file is not cached try to get it from Vault
// Get the private key from a vault path if defined in Githubapp spec
// Vault auth will take precedence over using `spec.privateKeySecret`
if githubApp.Spec.VaultPrivateKey != nil {
if githubApp.Spec.VaultPrivateKey != nil && len(privateKey) == 0 {

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")
Expand All @@ -453,12 +507,26 @@ func (r *GithubAppReconciler) generateOrUpdateAccessToken(ctx context.Context, g
if privateKeyErr != nil {
return fmt.Errorf("failed to get private key from vault: %v", privateKeyErr)
}
} else {
if len(privateKey) == 0 {
return fmt.Errorf("empty private key from vault")
}
// Cache the private key to file
if err := os.WriteFile(privateKeyPath, privateKey, 0600); err != nil {
return fmt.Errorf("failed to write private key to file: %v", err)
}
} else if githubApp.Spec.PrivateKeySecret != "" && len(privateKey) == 0 {
// else get the private key from K8s secret `spec.privateKeySecret`
privateKey, privateKeyErr = r.getPrivateKeyFromSecret(ctx, githubApp)
if privateKeyErr != nil {
return fmt.Errorf("failed to get private key from kubernetes secret: %v", privateKeyErr)
}
if len(privateKey) == 0 {
return fmt.Errorf("empty private key from vault")
}
// Cache the private key to file
if err := os.WriteFile(privateKeyPath, privateKey, 0600); err != nil {
return fmt.Errorf("failed to write private key to file: %v", err)
}
}

// Generate or renew access token
Expand All @@ -468,8 +536,12 @@ func (r *GithubAppReconciler) generateOrUpdateAccessToken(ctx context.Context, g
githubApp.Spec.InstallId,
privateKey,
)
// if GitHubApi request for access token fails
if err != nil {
return fmt.Errorf("failed to generate access token: %v", err)
// Delete private key cache
if err := deletePrivateKyCache(githubApp.Namespace, githubApp.Name); err != nil {
return err
}
}

// Create a new Secret with the access token
Expand Down Expand Up @@ -846,7 +918,11 @@ func getServiceAccountAndNamespace(serviceAccountPath string) (string, string, e
}

// SetupWithManager sets up the controller with the Manager.
func (r *GithubAppReconciler) SetupWithManager(mgr ctrl.Manager, tokenPath ...string) error {
func (r *GithubAppReconciler) SetupWithManager(mgr ctrl.Manager, privateKeyCache string, tokenPath ...string) error {

// Set private key cache path
privateKeyCachePath = privateKeyCache

// Get reconcile interval from environment variable or use default value
var err error
reconcileIntervalStr := os.Getenv("CHECK_INTERVAL")
Expand Down
13 changes: 11 additions & 2 deletions internal/controller/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ var (
testEnv *envtest.Environment
ctx context.Context
cancel context.CancelFunc
tokenFilePath = "/tmp/serviceAccountToken"
tokenFilePath = "/tmp/githubOperatorServiceAccountToken"
)

func TestControllers(t *testing.T) {
Expand Down Expand Up @@ -181,14 +181,20 @@ var _ = BeforeSuite(func() {
_, err = file.WriteString(token)
Expect(err).ToNot(HaveOccurred())

// Path to store private keys for local caching
privateKeyCachePath := "/tmp/github-app-operator/"
// Remove private key cache
err = os.RemoveAll(privateKeyCachePath)
Expect(err).NotTo(HaveOccurred())

err = (&GithubAppReconciler{
Client: k8sManager.GetClient(),
Scheme: k8sManager.GetScheme(),
Recorder: k8sManager.GetEventRecorderFor("githubapp-controller"),
HTTPClient: httpClient,
VaultClient: vaultClient,
K8sClient: k8sClientset,
}).SetupWithManager(k8sManager, tokenFilePath)
}).SetupWithManager(k8sManager, privateKeyCachePath, tokenFilePath)
Expect(err).ToNot(HaveOccurred())

go func() {
Expand All @@ -206,4 +212,7 @@ var _ = AfterSuite(func() {
// Remove service account token file
err = os.Remove(tokenFilePath)
Expect(err).NotTo(HaveOccurred())
// Remove private key cache
err = os.RemoveAll(privateKeyCachePath)
Expect(err).NotTo(HaveOccurred())
})

0 comments on commit ce3146c

Please sign in to comment.