From 935faf8475ed2b8dbf1a060a2ff715f638529e74 Mon Sep 17 00:00:00 2001 From: Karl Haworth <58607256+karlhaworth@users.noreply.github.com> Date: Fri, 30 Aug 2024 12:25:07 -0400 Subject: [PATCH] feat: allow webhook to use env vars or cert files for github app secret (#470) feat: allow webhook to use env vars or cert files for github app secret Builds on top of https://github.com/octo-sts/app/pull/412 --------- Signed-off-by: Karl Haworth --- cmd/app/main.go | 18 +++-- cmd/webhook/main.go | 76 ++++++++++--------- pkg/envconfig/envconfig.go | 36 ++++++++- pkg/envconfig/envconfig_test.go | 110 +++++++++++++++++++++++++--- pkg/ghtransport/ghtransport_test.go | 15 +--- 5 files changed, 187 insertions(+), 68 deletions(-) diff --git a/cmd/app/main.go b/cmd/app/main.go index 49a58a9..e0580d7 100644 --- a/cmd/app/main.go +++ b/cmd/app/main.go @@ -28,12 +28,16 @@ func main() { defer cancel() ctx = clog.WithLogger(ctx, clog.New(slog.Default().Handler())) - env, err := envConfig.Process() + baseCfg, err := envConfig.BaseConfig() + if err != nil { + log.Panicf("failed to process env var: %s", err) + } + appConfig, err := envConfig.AppConfig() if err != nil { log.Panicf("failed to process env var: %s", err) } - if env.Metrics { + if baseCfg.Metrics { go metrics.ServeMetrics() // Setup tracing. @@ -42,32 +46,32 @@ func main() { var client *kms.KeyManagementClient - if env.KMSKey != "" { + if baseCfg.KMSKey != "" { client, err = kms.NewKeyManagementClient(ctx) if err != nil { log.Panicf("could not create kms client: %v", err) } } - atr, err := ghtransport.New(ctx, env, client) + atr, err := ghtransport.New(ctx, baseCfg, client) if err != nil { log.Panicf("error creating GitHub App transport: %v", err) } d := duplex.New( - env.Port, + baseCfg.Port, // grpc.StatsHandler(otelgrpc.NewServerHandler()), // grpc.ChainStreamInterceptor(grpc_prometheus.StreamServerInterceptor), // grpc.ChainUnaryInterceptor(grpc_prometheus.UnaryServerInterceptor, interceptors.ServerErrorInterceptor), grpc.WithTransportCredentials(insecure.NewCredentials()), ) - ceclient, err := mce.NewClientHTTP("octo-sts", mce.WithTarget(ctx, env.EventingIngress)...) + ceclient, err := mce.NewClientHTTP("octo-sts", mce.WithTarget(ctx, appConfig.EventingIngress)...) if err != nil { log.Panicf("failed to create cloudevents client: %v", err) } - pboidc.RegisterSecurityTokenServiceServer(d.Server, octosts.NewSecurityTokenServiceServer(atr, ceclient, env.Domain, env.Metrics)) + pboidc.RegisterSecurityTokenServiceServer(d.Server, octosts.NewSecurityTokenServiceServer(atr, ceclient, appConfig.Domain, baseCfg.Metrics)) if err := d.RegisterHandler(ctx, pboidc.RegisterSecurityTokenServiceHandlerFromEndpoint); err != nil { log.Panicf("failed to register gateway endpoint: %v", err) } diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go index f4267bd..636c53a 100644 --- a/cmd/webhook/main.go +++ b/cmd/webhook/main.go @@ -17,62 +17,68 @@ import ( kms "cloud.google.com/go/kms/apiv1" secretmanager "cloud.google.com/go/secretmanager/apiv1" "cloud.google.com/go/secretmanager/apiv1/secretmanagerpb" - "github.com/bradleyfalzon/ghinstallation/v2" "github.com/chainguard-dev/clog" metrics "github.com/chainguard-dev/terraform-infra-common/pkg/httpmetrics" - "github.com/kelseyhightower/envconfig" - "github.com/octo-sts/app/pkg/gcpkms" + envConfig "github.com/octo-sts/app/pkg/envconfig" + "github.com/octo-sts/app/pkg/ghtransport" "github.com/octo-sts/app/pkg/webhook" ) -type envConfig struct { - Port int `envconfig:"PORT" required:"true" default:"8080"` - KMSKey string `envconfig:"KMS_KEY" required:"true"` - AppID int64 `envconfig:"GITHUB_APP_ID" required:"true"` - WebhookSecret string `envconfig:"GITHUB_WEBHOOK_SECRET" required:"true"` -} - func main() { ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt) defer cancel() ctx = clog.WithLogger(ctx, clog.New(slog.Default().Handler())) - go metrics.ServeMetrics() - - // Setup tracing. - defer metrics.SetupTracer(ctx)() - - var env envConfig - if err := envconfig.Process("", &env); err != nil { + baseCfg, err := envConfig.BaseConfig() + if err != nil { log.Panicf("failed to process env var: %s", err) } - - kms, err := kms.NewKeyManagementClient(ctx) + webhookConfig, err := envConfig.WebhookConfig() if err != nil { - log.Panicf("could not create kms client: %v", err) + log.Panicf("failed to process env var: %s", err) } - signer, err := gcpkms.New(ctx, kms, env.KMSKey) - if err != nil { - log.Panicf("error creating signer: %v", err) + + if baseCfg.Metrics { + go metrics.ServeMetrics() + + // Setup tracing. + defer metrics.SetupTracer(ctx)() } - atr, err := ghinstallation.NewAppsTransportWithOptions(http.DefaultTransport, env.AppID, ghinstallation.WithSigner(signer)) + + var client *kms.KeyManagementClient + + if baseCfg.KMSKey != "" { + client, err = kms.NewKeyManagementClient(ctx) + if err != nil { + log.Panicf("could not create kms client: %v", err) + } + } + + atr, err := ghtransport.New(ctx, baseCfg, client) if err != nil { log.Panicf("error creating GitHub App transport: %v", err) } + // Fetch webhook secrets from secret manager + // or allow webhook secret to be defined by env var. + // Not everyone is using Google KMS, so we need to support other methods webhookSecrets := [][]byte{} - secretmanager, err := secretmanager.NewClient(ctx) - if err != nil { - log.Panicf("could not create secret manager client: %v", err) - } - for _, name := range strings.Split(env.WebhookSecret, ",") { - resp, err := secretmanager.AccessSecretVersion(ctx, &secretmanagerpb.AccessSecretVersionRequest{ - Name: name, - }) + if baseCfg.KMSKey != "" { + secretmanager, err := secretmanager.NewClient(ctx) if err != nil { - log.Panicf("error fetching webhook secret %s: %v", name, err) + log.Panicf("could not create secret manager client: %v", err) + } + for _, name := range strings.Split(webhookConfig.WebhookSecret, ",") { + resp, err := secretmanager.AccessSecretVersion(ctx, &secretmanagerpb.AccessSecretVersionRequest{ + Name: name, + }) + if err != nil { + log.Panicf("error fetching webhook secret %s: %v", name, err) + } + webhookSecrets = append(webhookSecrets, resp.GetPayload().GetData()) } - webhookSecrets = append(webhookSecrets, resp.GetPayload().GetData()) + } else { + webhookSecrets = [][]byte{[]byte(webhookConfig.WebhookSecret)} } mux := http.NewServeMux() @@ -81,7 +87,7 @@ func main() { WebhookSecret: webhookSecrets, }) srv := &http.Server{ - Addr: fmt.Sprintf(":%d", env.Port), + Addr: fmt.Sprintf(":%d", baseCfg.Port), ReadHeaderTimeout: 10 * time.Second, Handler: mux, } diff --git a/pkg/envconfig/envconfig.go b/pkg/envconfig/envconfig.go index 0915296..bfa1713 100644 --- a/pkg/envconfig/envconfig.go +++ b/pkg/envconfig/envconfig.go @@ -11,17 +11,47 @@ import ( type EnvConfig struct { Port int `envconfig:"PORT" required:"true"` - Domain string `envconfig:"STS_DOMAIN" required:"true"` KMSKey string `envconfig:"KMS_KEY" required:"false"` AppID int64 `envconfig:"GITHUB_APP_ID" required:"true"` - EventingIngress string `envconfig:"EVENT_INGRESS_URI" required:"true"` AppSecretCertificateFile string `envconfig:"APP_SECRET_CERTIFICATE_FILE" required:"false"` AppSecretCertificateEnvVar string `envconfig:"APP_SECRET_CERTIFICATE_ENV_VAR" required:"false"` Metrics bool `envconfig:"METRICS" required:"false" default:"true"` } -func Process() (*EnvConfig, error) { +type EnvConfigApp struct { + Domain string `envconfig:"STS_DOMAIN" required:"true"` + EventingIngress string `envconfig:"EVENT_INGRESS_URI" required:"true"` +} + +type EnvConfigWebhook struct { + WebhookSecret string `envconfig:"WEBHOOK_SECRET" required:"true"` +} + +func AppConfig() (*EnvConfigApp, error) { + cfg := new(EnvConfigApp) + + var err error + if err = envconfig.Process("", cfg); err != nil { + return nil, err + } + + return cfg, err +} + +func WebhookConfig() (*EnvConfigWebhook, error) { + cfg := new(EnvConfigWebhook) + + var err error + if err = envconfig.Process("", cfg); err != nil { + return nil, err + } + + return cfg, err +} + +func BaseConfig() (*EnvConfig, error) { cfg := new(EnvConfig) + var err error if err = envconfig.Process("", cfg); err != nil { return nil, err diff --git a/pkg/envconfig/envconfig_test.go b/pkg/envconfig/envconfig_test.go index a1f7837..f02312a 100644 --- a/pkg/envconfig/envconfig_test.go +++ b/pkg/envconfig/envconfig_test.go @@ -9,7 +9,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestProcess(t *testing.T) { +func TestBaseConfig(t *testing.T) { tests := []struct { name string envVars map[string]string @@ -19,9 +19,7 @@ func TestProcess(t *testing.T) { name: "No environment variables set", envVars: map[string]string{ "PORT": "8080", - "STS_DOMAIN": "", "GITHUB_APP_ID": "1234", - "EVENT_INGRESS_URI": "", "KMS_KEY": "", "APP_SECRET_CERTIFICATE_FILE": "", "APP_SECRET_CERTIFICATE_ENVVAR": "", @@ -32,9 +30,7 @@ func TestProcess(t *testing.T) { name: "Only KMS_KEY set", envVars: map[string]string{ "PORT": "8080", - "STS_DOMAIN": "", "GITHUB_APP_ID": "1234", - "EVENT_INGRESS_URI": "", "KMS_KEY": "some-kms-key", "APP_SECRET_CERTIFICATE_FILE": "", "APP_SECRET_CERTIFICATE_ENVVAR": "", @@ -45,9 +41,7 @@ func TestProcess(t *testing.T) { name: "Only APP_SECRET_CERTIFICATE_FILE set", envVars: map[string]string{ "PORT": "8080", - "STS_DOMAIN": "", "GITHUB_APP_ID": "1234", - "EVENT_INGRESS_URI": "", "KMS_KEY": "", "APP_SECRET_CERTIFICATE_FILE": "some-file-path", "APP_SECRET_CERTIFICATE_ENVVAR": "", @@ -58,9 +52,7 @@ func TestProcess(t *testing.T) { name: "Only APP_SECRET_CERTIFICATE_ENVVAR set", envVars: map[string]string{ "PORT": "8080", - "STS_DOMAIN": "", "GITHUB_APP_ID": "1234", - "EVENT_INGRESS_URI": "", "KMS_KEY": "", "APP_SECRET_CERTIFICATE_FILE": "", "APP_SECRET_CERTIFICATE_ENVVAR": "some-env-var", @@ -71,9 +63,7 @@ func TestProcess(t *testing.T) { name: "Multiple variables set", envVars: map[string]string{ "PORT": "8080", - "STS_DOMAIN": "", "GITHUB_APP_ID": "1234", - "EVENT_INGRESS_URI": "", "KMS_KEY": "some-kms-key", "APP_SECRET_CERTIFICATE_FILE": "some-file-path", "APP_SECRET_CERTIFICATE_ENVVAR": "", @@ -88,7 +78,103 @@ func TestProcess(t *testing.T) { t.Setenv(key, value) } - cfg, err := Process() + cfg, err := BaseConfig() + + if tt.wantErr { + assert.Error(t, err) + assert.Nil(t, cfg) + } else { + assert.NoError(t, err) + assert.NotNil(t, cfg) + } + }) + } +} + +func TestAppConfig(t *testing.T) { + tests := []struct { + name string + envVars map[string]string + wantErr bool + }{ + { + name: "No environment variables set", + envVars: map[string]string{ + "STS_DOMAIN": "", + "EVENT_INGRESS_URI": "", + }, + wantErr: false, + }, + { + name: "All environment variables set", + envVars: map[string]string{ + "STS_DOMAIN": "octo-sts-test.local", + "EVENT_INGRESS_URI": "http://localhost:8082", + }, + wantErr: false, + }, + { + name: "Missing Event Ingress URI", + envVars: map[string]string{ + "STS_DOMAIN": "octo-sts-test.local", + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for key, value := range tt.envVars { + t.Setenv(key, value) + } + + cfg, err := AppConfig() + + if tt.wantErr { + assert.Error(t, err) + assert.Nil(t, cfg) + } else { + assert.NoError(t, err) + assert.NotNil(t, cfg) + } + }) + } +} + +func TestWebhookConfig(t *testing.T) { + tests := []struct { + name string + envVars map[string]string + wantErr bool + }{ + { + name: "No environment variables set", + envVars: map[string]string{ + "WEBHOOK_SECRET": "", + }, + wantErr: false, + }, + { + name: "All environment variables set", + envVars: map[string]string{ + "WEBHOOK_SECRET": "octo-sts-test.local", + }, + wantErr: false, + }, + { + name: "Missing Event Ingress URI", + envVars: map[string]string{}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for key, value := range tt.envVars { + t.Setenv(key, value) + } + + cfg, err := WebhookConfig() if tt.wantErr { assert.Error(t, err) diff --git a/pkg/ghtransport/ghtransport_test.go b/pkg/ghtransport/ghtransport_test.go index 3f1544e..8d13e3f 100644 --- a/pkg/ghtransport/ghtransport_test.go +++ b/pkg/ghtransport/ghtransport_test.go @@ -34,12 +34,10 @@ func TestGCPKMS(t *testing.T) { t.Setenv("GOOGLE_APPLICATION_CREDENTIALS", credsFile) testConfig := &envconfig.EnvConfig{ - Port: 8080, - Domain: "example.com", - AppID: 123456, - EventingIngress: "https://event.ingress.uri", - KMSKey: "test-kms-key", - Metrics: true, + Port: 8080, + AppID: 123456, + KMSKey: "test-kms-key", + Metrics: true, } transport, err := New(ctx, testConfig, generateKMSClient(ctx, t)) @@ -56,9 +54,7 @@ func TestCertEnvVar(t *testing.T) { testConfig := &envconfig.EnvConfig{ Port: 8080, - Domain: "example.com", AppID: 123456, - EventingIngress: "https://event.ingress.uri", AppSecretCertificateEnvVar: "GITHUB_APP_SECRET", Metrics: true, } @@ -75,9 +71,7 @@ func TestCertFile(t *testing.T) { testConfig := &envconfig.EnvConfig{ Port: 8080, - Domain: "example.com", AppID: 123456, - EventingIngress: "https://event.ingress.uri", AppSecretCertificateFile: generateTestCertificateFile(t), Metrics: true, } @@ -125,7 +119,6 @@ func createGCPKMSCredsFile(t *testing.T) string { if err := tmpFile.Close(); err != nil { t.Fatalf("Failed to close temporary file: %s", err) } - t.Logf(tmpFile.Name()) return tmpFile.Name() }