feat(runner): allow for optional environment variables (#443)

Signed-off-by: Mattia Buccarella <[email protected]>

Author: buccarel

app/cli/internal/action/attestation_init.go

@@ -123,7 +123,11 @@ func (action *AttestationInit) Run(contractRevision int) error {
 	}
 
 	// Load the env variables both the system populated and the user predefined ones
-	if err := action.c.ResolveEnvVars(!action.dryRun); err != nil {
+	if err := action.c.ResolveEnvVars(); err != nil {
+		if action.dryRun {
+			return nil
+		}
+
 		_ = action.c.Reset()
 		return err
 	}

app/cli/internal/action/attestation_status.go

@@ -124,8 +124,18 @@ func (action *AttestationStatus) Run() (*AttestationStatusResult, error) {
 	}
 
 	res.EnvVars = envVars
+
+	runnerEnvVars, errors := c.Runner.ResolveEnvVars()
+	var combinedErrs string
+	for _, err := range errors {
+		combinedErrs += (*err).Error() + "\n"
+	}
+	if len(errors) > 0 && !c.CraftingState.DryRun {
+		return nil, fmt.Errorf("error resolving env vars: %s", combinedErrs)
+	}
+
 	res.RunnerContext = &AttestationResultRunnerContext{
-		EnvVars:    c.Runner.ResolveEnvVars(),
+		EnvVars:    runnerEnvVars,
 		RunnerType: att.RunnerType.String(),
 		JobURL:     att.RunnerUrl,
 	}

internal/attestation/crafter/crafter.go

@@ -372,49 +372,44 @@ func persistCraftingState(craftState *api.CraftingState, stateFilePath string) e
 
 // ResolveEnvVars will iterate on the env vars in the allow list and resolve them from the system context
 // strict indicates if it should fail if any env variable can not be found
-func (c *Crafter) ResolveEnvVars(strict bool) error {
+func (c *Crafter) ResolveEnvVars() error {
 	if err := c.requireStateLoaded(); err != nil {
 		return err
 	}
 
-	// Runner specific env variables
+	// Runner specific environment variables
 	c.logger.Debug().Str("runnerType", c.Runner.String()).Msg("loading runner specific env variables")
-	var outputEnvVars = make(map[string]string)
 	if !c.Runner.CheckEnv() {
 		errorStr := fmt.Sprintf("couldn't detect the environment %q. Is the crafting process happening in the target env?", c.Runner.String())
-		if strict {
-			return fmt.Errorf("%s - %w", errorStr, ErrRunnerContextNotFound)
-		}
-		c.logger.Warn().Msg(errorStr)
-	} else {
-		c.logger.Debug().Str("runnerType", c.Runner.String()).Strs("variables", c.Runner.ListEnvVars()).Msg("list of env variables to automatically extract")
-		outputEnvVars = c.Runner.ResolveEnvVars()
-		if notFound := notResolvedVars(outputEnvVars, c.Runner.ListEnvVars()); len(notFound) > 0 {
-			if strict {
-				return fmt.Errorf("required env variables not present %q", notFound)
-			}
-			c.logger.Warn().Strs("key", notFound).Msg("required env variables not present")
-		}
+		return fmt.Errorf("%s - %w", errorStr, ErrRunnerContextNotFound)
 	}
 
-	// User-defined env vars
-	varsAllowList := c.CraftingState.InputSchema.EnvAllowList
-	if len(varsAllowList) > 0 {
-		c.logger.Debug().Strs("allowList", varsAllowList).Msg("loading env variables")
-		for _, want := range varsAllowList {
-			val := os.Getenv(want)
-			if val == "" {
-				continue
-			}
+	// Workflow run environment variables
+	varNames := make([]string, len(c.Runner.ListEnvVars()))
+	for index, envVarDef := range c.Runner.ListEnvVars() {
+		varNames[index] = envVarDef.Name
+	}
+	c.logger.Debug().Str("runnerType", c.Runner.String()).Strs("variables", varNames).Msg("list of env variables to automatically extract")
 
-			outputEnvVars[want] = val
+	outputEnvVars, errors := c.Runner.ResolveEnvVars()
+	if len(errors) > 0 {
+		var combinedErrs string
+		for _, err := range errors {
+			combinedErrs += (*err).Error() + "\n"
 		}
+		return fmt.Errorf("error while resolving runner environment variables: %s", combinedErrs)
+	}
 
-		if notFound := notResolvedVars(outputEnvVars, varsAllowList); len(notFound) > 0 {
-			if strict {
-				return fmt.Errorf("required env variables not present %q", notFound)
-			}
-			c.logger.Warn().Strs("key", notFound).Msg("required env variables not present")
+	// User-defined environment vars
+	if len(c.CraftingState.InputSchema.EnvAllowList) > 0 {
+		c.logger.Debug().Strs("allowList", c.CraftingState.InputSchema.EnvAllowList).Msg("loading env variables")
+	}
+	for _, want := range c.CraftingState.InputSchema.EnvAllowList {
+		val := os.Getenv(want)
+		if val != "" {
+			outputEnvVars[want] = val
+		} else {
+			return fmt.Errorf("required env variables not present %q", want)
 		}
 	}
 
@@ -427,17 +422,6 @@ func (c *Crafter) ResolveEnvVars(strict bool) error {
 	return nil
 }
 
-func notResolvedVars(resolved map[string]string, wantList []string) []string {
-	var notFound []string
-	for _, want := range wantList {
-		if val, ok := resolved[want]; !ok || val == "" {
-			notFound = append(notFound, want)
-		}
-	}
-
-	return notFound
-}
-
 // Inject material to attestation state
 func (c *Crafter) AddMaterial(key, value string, casBackend *casclient.CASBackend, runtimeAnnotations map[string]string) error {
 	if err := c.requireStateLoaded(); err != nil {

internal/attestation/crafter/crafter_test.go

@@ -221,138 +221,108 @@ func (s *crafterSuite) TestLoadSchema() {
 
 func (s *crafterSuite) TestResolveEnvVars() {
 	testCases := []struct {
-		name   string
-		strict bool
+		name string
+
 		// Custom env variables to expose
 		envVars map[string]string
-		// Simulate that the crafting process is hapenning in a github action runner
-		inGithubEnv bool
-		wantErr     bool
-		// Total list of resolved env vars
-		want map[string]string
+
+		// Simulate that the crafting process is hapenning in a specific runner
+		inGithubEnv  bool
+		inJenkinsEnv bool
+
+		expectedError   string
+		expectedEnvVars map[string]string
 	}{
 		{
-			name:        "strict missing custom vars",
-			strict:      true,
-			inGithubEnv: true,
-			wantErr:     true,
-		},
-		{
-			name:        "strict, not running in github env",
-			strict:      true,
-			inGithubEnv: false,
-			wantErr:     true,
+			name:          "missing custom vars",
+			inGithubEnv:   true,
+			expectedError: "required env variables not present \"CUSTOM_VAR_1\"",
+		}, {
+			name:          "missing some github runner env",
+			inGithubEnv:   true,
+			expectedError: "error while resolving runner environment variables: environment variable GITHUB_ACTOR cannot be resolved\n",
 			envVars: map[string]string{
 				"CUSTOM_VAR_1": "custom_value_1",
 				"CUSTOM_VAR_2": "custom_value_2",
+				"GITHUB_ACTOR": "", // This is removing one necessary variable
 			},
-		},
-		{
-			name:        "strict, missing some github env",
-			strict:      true,
-			inGithubEnv: false,
-			wantErr:     true,
-			envVars: map[string]string{
-				"CUSTOM_VAR_1":  "custom_value_1",
-				"CUSTOM_VAR_2":  "custom_value_2",
-				"CI":            "true",
-				"GITHUB_RUN_ID": "123",
-			},
-		},
-		{
-			name:        "strict and all envs available",
-			strict:      true,
-			inGithubEnv: true,
+		}, {
+			name:          "missing optional jenkins variable with no error",
+			inJenkinsEnv:  true,
+			expectedError: "",
 			envVars: map[string]string{
+				// Missing var: GIT_BRANCH
 				"CUSTOM_VAR_1": "custom_value_1",
 				"CUSTOM_VAR_2": "custom_value_2",
 			},
-			want: map[string]string{
-				"CUSTOM_VAR_1":            "custom_value_1",
-				"CUSTOM_VAR_2":            "custom_value_2",
-				"GITHUB_REPOSITORY":       "chainloop/chainloop",
-				"GITHUB_RUN_ID":           "123",
-				"GITHUB_ACTOR":            "chainloop",
-				"GITHUB_REF":              "refs/heads/main",
-				"GITHUB_REPOSITORY_OWNER": "chainloop",
-				"GITHUB_SHA":              "1234567890",
-				"RUNNER_NAME":             "chainloop-runner",
-				"RUNNER_OS":               "linux",
-			},
-		},
-		{
-			name:        "non strict missing custom vars",
-			strict:      false,
-			inGithubEnv: true,
-			wantErr:     false,
-			want:        gitHubTestingEnvVars,
-		},
-		{
-			name:        "non strict, missing some github env",
-			strict:      false,
-			inGithubEnv: false,
-			wantErr:     false,
-			envVars: map[string]string{
-				"CUSTOM_VAR_1":      "custom_value_1",
-				"CUSTOM_VAR_2":      "custom_value_2",
-				"CI":                "true",
-				"GITHUB_RUN_ID":     "123",
-				"GITHUB_REPOSITORY": "chainloop/chainloop",
-			},
-			want: map[string]string{
-				"CUSTOM_VAR_1":            "custom_value_1",
-				"CUSTOM_VAR_2":            "custom_value_2",
-				"GITHUB_REPOSITORY":       "chainloop/chainloop",
-				"GITHUB_RUN_ID":           "123",
-				"GITHUB_ACTOR":            "",
-				"GITHUB_REF":              "",
-				"GITHUB_REPOSITORY_OWNER": "",
-				"GITHUB_SHA":              "",
-				"RUNNER_NAME":             "",
-				"RUNNER_OS":               "",
+			expectedEnvVars: map[string]string{
+				// Missing var: GIT_BRANCH
+				"CUSTOM_VAR_1":  "custom_value_1",
+				"CUSTOM_VAR_2":  "custom_value_2",
+				"JOB_NAME":      "some-job",
+				"BUILD_URL":     "http://some-url",
+				"AGENT_WORKDIR": "/some/home/dir",
+				"NODE_NAME":     "some-node",
 			},
-		},
-		{
-			name:        "non strict, wrong runner context",
-			strict:      false,
-			inGithubEnv: false,
-			wantErr:     false,
+		}, {
+			name:          "all optional jenkins variable with no error",
+			inJenkinsEnv:  true,
+			expectedError: "",
 			envVars: map[string]string{
+				"GIT_BRANCH":   "some-branch", // optional var 1
+				"GIT_COMMIT":   "some-commit", // optional var 2
 				"CUSTOM_VAR_1": "custom_value_1",
 				"CUSTOM_VAR_2": "custom_value_2",
 			},
-			want: map[string]string{
-				"CUSTOM_VAR_1": "custom_value_1",
-				"CUSTOM_VAR_2": "custom_value_2",
+			expectedEnvVars: map[string]string{
+				"GIT_BRANCH":    "some-branch", // optional var 1
+				"GIT_COMMIT":    "some-commit", // optional var 2
+				"CUSTOM_VAR_1":  "custom_value_1",
+				"CUSTOM_VAR_2":  "custom_value_2",
+				"JOB_NAME":      "some-job",
+				"BUILD_URL":     "http://some-url",
+				"AGENT_WORKDIR": "/some/home/dir",
+				"NODE_NAME":     "some-node",
 			},
 		},
 	}
 
 	for _, tc := range testCases {
 		s.Run(tc.name, func() {
-			// Customs env vars
-			for k, v := range tc.envVars {
-				s.T().Setenv(k, v)
-			}
-			// Runner env variables
+			contract := "testdata/contracts/with_env_vars.yaml"
 			if tc.inGithubEnv {
 				s.T().Setenv("CI", "true")
 				for k, v := range gitHubTestingEnvVars {
 					s.T().Setenv(k, v)
 				}
+			} else if tc.inJenkinsEnv {
+				contract = "testdata/contracts/jenkins_with_env_vars.yaml"
+				s.T().Setenv("JOB_NAME", "some-job")
+				s.T().Setenv("BUILD_URL", "http://some-url")
+				s.T().Setenv("AGENT_WORKDIR", "/some/home/dir")
+				s.T().Setenv("NODE_NAME", "some-node")
+				s.T().Setenv("JENKINS_HOME", "/some/home/dir")
 			}
 
-			c, err := newInitializedCrafter(s.T(), "testdata/contracts/with_env_vars.yaml", &v1.WorkflowMetadata{}, true, "")
+			// Customs env vars
+			for k, v := range tc.envVars {
+				s.T().Setenv(k, v)
+			}
+
+			c, err := newInitializedCrafter(s.T(), contract, &v1.WorkflowMetadata{}, false, "")
 			require.NoError(s.T(), err)
 
-			err = c.ResolveEnvVars(tc.strict)
-			if tc.wantErr {
+			err = c.ResolveEnvVars()
+
+			if tc.expectedError != "" {
 				s.Error(err)
+				actualError := err.Error()
+				s.Equal(tc.expectedError, actualError)
 				return
 			}
 
 			s.NoError(err)
-			s.Equal(tc.want, c.CraftingState.Attestation.EnvVars)
+			s.Equal(tc.expectedEnvVars, c.CraftingState.Attestation.EnvVars)
 		})
 	}
 }

internal/attestation/crafter/runner.go

@@ -27,11 +27,15 @@ var ErrRunnerContextNotFound = errors.New("the runner environment doesn't match
 type supportedRunner interface {
 	// Whether the attestation is happening in this environment
 	CheckEnv() bool
+
 	// List the env variables registered
-	ListEnvVars() []string
+	ListEnvVars() []*runners.EnvVarDefinition
+
 	// Return the list of env vars associated with this runner already resolved
-	ResolveEnvVars() map[string]string
+	ResolveEnvVars() (map[string]string, []*error)
+
 	String() string
+
 	// uri to the running job/workload
 	RunURI() string
 }

internal/attestation/crafter/runners/azurepipeline.go

@@ -40,25 +40,21 @@ func (r *AzurePipeline) CheckEnv() bool {
 	return true
 }
 
-func (r *AzurePipeline) ListEnvVars() []string {
-	return []string{
-		"BUILD_REQUESTEDFOREMAIL",
-		"BUILD_REQUESTEDFOR",
-		"BUILD_REPOSITORY_URI",
-		"BUILD_REPOSITORY_NAME",
-		"BUILD_BUILDID",
-		"BUILD_BUILDNUMBER",
-		"BUILD_BUILDURI",
-		"BUILD_REASON",
-		"AGENT_VERSION",
-		"TF_BUILD",
+func (r *AzurePipeline) ListEnvVars() []*EnvVarDefinition {
+	return []*EnvVarDefinition{
+		{"BUILD_REQUESTEDFOREMAIL", false},
+		{"BUILD_REQUESTEDFOR", false},
+		{"BUILD_REPOSITORY_URI", false},
+		{"BUILD_REPOSITORY_NAME", false},
+		{"BUILD_BUILDID", false},
+		{"BUILD_BUILDNUMBER", false},
+		{"BUILD_BUILDURI", false},
+		{"BUILD_REASON", false},
+		{"AGENT_VERSION", false},
+		{"TF_BUILD", false},
 	}
 }
 
-func (r *AzurePipeline) ResolveEnvVars() map[string]string {
-	return resolveEnvVars(r.ListEnvVars())
-}
-
 func (r *AzurePipeline) String() string {
 	return AzurePipelineID
 }
@@ -84,3 +80,7 @@ func (r *AzurePipeline) RunURI() (url string) {
 
 	return uri.String()
 }
+
+func (r *AzurePipeline) ResolveEnvVars() (map[string]string, []*error) {
+	return resolveEnvVars(r.ListEnvVars())
+}