From 215bd71855ee68846fa01ba2b7b38dccb74cb92b Mon Sep 17 00:00:00 2001 From: Mario Constanti Date: Thu, 30 Nov 2023 16:40:46 +0100 Subject: [PATCH] feat: passthrough additional env vars to provider as some provider binaries probably need additional environment variables set (e.g kubernetes as client-go depends on KUBERNETES_SERVICE_ vars) it should be possible to define a list of environment variables which should get bypassed into the provider binary execution --- config/config.go | 3 + config/external.go | 20 ++++++ config/external_test.go | 99 +++++++++++++++++++++++++++ runner/providers/external/external.go | 31 ++++++--- 4 files changed, 145 insertions(+), 8 deletions(-) diff --git a/config/config.go b/config/config.go index c2a12017..15c349da 100644 --- a/config/config.go +++ b/config/config.go @@ -39,6 +39,9 @@ const ( MySQLBackend DBBackendType = "mysql" // SQLiteBackend represents the SQLite3 DB backend SQLiteBackend DBBackendType = "sqlite3" + // EnvironmentVariablePrefix is the prefix for all environment variables + // that can not be used to get overwritten via the external provider + EnvironmentVariablePrefix = "GARM" ) // NewConfig returns a new Config diff --git a/config/external.go b/config/external.go index 5bd9e273..8368b13c 100644 --- a/config/external.go +++ b/config/external.go @@ -18,6 +18,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "github.com/cloudbase/garm-provider-common/util/exec" @@ -42,6 +43,25 @@ type External struct { // the provider. If specified, it will take precedence over the "garm-external-provider" // executable in the ProviderDir. ProviderExecutable string `toml:"provider_executable" json:"provider-executable"` + // EnvironmentVariables is a list of environment variable names that will be + // passed to the external binary together with their values. + EnvironmentVariables []string `toml:"environment_variables" json:"environment-variables"` +} + +func (e *External) GetEnvironmentVariables() []string { + envVars := []string{} + + for _, configuredEnvVars := range e.EnvironmentVariables { + // discover environment variables + for _, k := range os.Environ() { + variable := strings.SplitN(k, "=", 2) + if strings.HasPrefix(variable[0], configuredEnvVars) && + !strings.HasPrefix(variable[0], EnvironmentVariablePrefix) { + envVars = append(envVars, k) + } + } + } + return envVars } func (e *External) ExecutablePath() (string, error) { diff --git a/config/external_test.go b/config/external_test.go index 1da36d33..30085726 100644 --- a/config/external_test.go +++ b/config/external_test.go @@ -18,6 +18,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "testing" "github.com/stretchr/testify/require" @@ -121,3 +122,101 @@ func TestProviderExecutableIsExecutable(t *testing.T) { require.NotNil(t, err) require.EqualError(t, err, fmt.Sprintf("external provider binary %s is not executable", execPath)) } + +func TestExternalEnvironmentVariables(t *testing.T) { + cfg := getDefaultExternalConfig(t) + + tests := []struct { + name string + cfg External + expectedEnvironmentVariables []string + environmentVariables map[string]string + }{ + { + name: "Provider with no additional environment variables", + cfg: cfg, + expectedEnvironmentVariables: []string{}, + environmentVariables: map[string]string{ + "PATH": "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "PROVIDER_LOG_LEVEL": "debug", + "PROVIDER_TIMEOUT": "30", + "PROVIDER_RETRY_COUNT": "3", + "INFRA_REGION": "us-east-1", + }, + }, + { + name: "Provider with additional environment variables", + cfg: External{ + ConfigFile: "", + ProviderDir: "../test", + EnvironmentVariables: []string{ + "PROVIDER_", + "INFRA_REGION", + }, + }, + environmentVariables: map[string]string{ + "PATH": "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "PROVIDER_LOG_LEVEL": "debug", + "PROVIDER_TIMEOUT": "30", + "PROVIDER_RETRY_COUNT": "3", + "INFRA_REGION": "us-east-1", + "GARM_POOL_ID": "f3b21376-e189-43ae-a1bd-7a3ffee57a58", + }, + expectedEnvironmentVariables: []string{ + "PROVIDER_LOG_LEVEL=debug", + "PROVIDER_TIMEOUT=30", + "PROVIDER_RETRY_COUNT=3", + "INFRA_REGION=us-east-1", + }, + }, + { + name: "GARM variables are getting ignored", + cfg: External{ + ConfigFile: "", + ProviderDir: "../test", + EnvironmentVariables: []string{ + "PROVIDER_", + "INFRA_REGION", + "GARM_SERVER", + }, + }, + environmentVariables: map[string]string{ + "PATH": "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "PROVIDER_LOG_LEVEL": "debug", + "PROVIDER_TIMEOUT": "30", + "PROVIDER_RETRY_COUNT": "3", + "INFRA_REGION": "us-east-1", + "GARM_POOL_ID": "f3b21376-e189-43ae-a1bd-7a3ffee57a58", + "GARM_SERVER_SHUTDOWN": "true", + "GARM_SERVER_INSECURE": "true", + }, + expectedEnvironmentVariables: []string{ + "PROVIDER_LOG_LEVEL=debug", + "PROVIDER_TIMEOUT=30", + "PROVIDER_RETRY_COUNT=3", + "INFRA_REGION=us-east-1", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // set environment variables + for k, v := range tc.environmentVariables { + err := os.Setenv(k, v) + if err != nil { + t.Fatalf("failed to set environment variable: %s", err) + } + } + + envVars := tc.cfg.GetEnvironmentVariables() + + // sort slices to make them comparable + slices.Sort(envVars) + slices.Sort(tc.expectedEnvironmentVariables) + + // compare slices + require.Equal(t, tc.expectedEnvironmentVariables, envVars) + }) + } +} diff --git a/runner/providers/external/external.go b/runner/providers/external/external.go index c1bd7141..3e58e0cd 100644 --- a/runner/providers/external/external.go +++ b/runner/providers/external/external.go @@ -31,19 +31,24 @@ func NewProvider(ctx context.Context, cfg *config.Provider, controllerID string) if err != nil { return nil, errors.Wrap(err, "fetching executable path") } + + envVars := cfg.External.GetEnvironmentVariables() + return &external{ - ctx: ctx, - controllerID: controllerID, - cfg: cfg, - execPath: execPath, + ctx: ctx, + controllerID: controllerID, + cfg: cfg, + execPath: execPath, + environmentVariables: envVars, }, nil } type external struct { - ctx context.Context - controllerID string - cfg *config.Provider - execPath string + ctx context.Context + controllerID string + cfg *config.Provider + execPath string + environmentVariables []string } func (e *external) validateResult(inst commonParams.ProviderInstance) error { @@ -74,6 +79,7 @@ func (e *external) CreateInstance(ctx context.Context, bootstrapParams commonPar fmt.Sprintf("GARM_POOL_ID=%s", bootstrapParams.PoolID), fmt.Sprintf("GARM_PROVIDER_CONFIG_FILE=%s", e.cfg.External.ConfigFile), } + asEnv = append(asEnv, e.environmentVariables...) asJs, err := json.Marshal(bootstrapParams) if err != nil { @@ -107,6 +113,7 @@ func (e *external) DeleteInstance(ctx context.Context, instance string) error { fmt.Sprintf("GARM_INSTANCE_ID=%s", instance), fmt.Sprintf("GARM_PROVIDER_CONFIG_FILE=%s", e.cfg.External.ConfigFile), } + asEnv = append(asEnv, e.environmentVariables...) _, err := garmExec.Exec(ctx, e.execPath, nil, asEnv) if err != nil { @@ -127,6 +134,7 @@ func (e *external) GetInstance(ctx context.Context, instance string) (commonPara fmt.Sprintf("GARM_INSTANCE_ID=%s", instance), fmt.Sprintf("GARM_PROVIDER_CONFIG_FILE=%s", e.cfg.External.ConfigFile), } + asEnv = append(asEnv, e.environmentVariables...) // TODO(gabriel-samfira): handle error types. Of particular insterest is to // know when the error is ErrNotFound. @@ -155,6 +163,7 @@ func (e *external) ListInstances(ctx context.Context, poolID string) ([]commonPa fmt.Sprintf("GARM_POOL_ID=%s", poolID), fmt.Sprintf("GARM_PROVIDER_CONFIG_FILE=%s", e.cfg.External.ConfigFile), } + asEnv = append(asEnv, e.environmentVariables...) out, err := garmExec.Exec(ctx, e.execPath, nil, asEnv) if err != nil { @@ -183,6 +192,8 @@ func (e *external) RemoveAllInstances(ctx context.Context) error { fmt.Sprintf("GARM_CONTROLLER_ID=%s", e.controllerID), fmt.Sprintf("GARM_PROVIDER_CONFIG_FILE=%s", e.cfg.External.ConfigFile), } + asEnv = append(asEnv, e.environmentVariables...) + _, err := garmExec.Exec(ctx, e.execPath, nil, asEnv) if err != nil { return garmErrors.NewProviderError("provider binary %s returned error: %s", e.execPath, err) @@ -198,6 +209,8 @@ func (e *external) Stop(ctx context.Context, instance string, force bool) error fmt.Sprintf("GARM_INSTANCE_ID=%s", instance), fmt.Sprintf("GARM_PROVIDER_CONFIG_FILE=%s", e.cfg.External.ConfigFile), } + asEnv = append(asEnv, e.environmentVariables...) + _, err := garmExec.Exec(ctx, e.execPath, nil, asEnv) if err != nil { return garmErrors.NewProviderError("provider binary %s returned error: %s", e.execPath, err) @@ -213,6 +226,8 @@ func (e *external) Start(ctx context.Context, instance string) error { fmt.Sprintf("GARM_INSTANCE_ID=%s", instance), fmt.Sprintf("GARM_PROVIDER_CONFIG_FILE=%s", e.cfg.External.ConfigFile), } + asEnv = append(asEnv, e.environmentVariables...) + _, err := garmExec.Exec(ctx, e.execPath, nil, asEnv) if err != nil { return garmErrors.NewProviderError("provider binary %s returned error: %s", e.execPath, err)