Skip to content

ci-operator: allow multi-stage params to opt into trigger-time overrides #5377

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Prucek wants to merge 1 commit into
base: main
Choose a base branch
from
Open

ci-operator: allow multi-stage params to opt into trigger-time overrides #5377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 67 additions & 12 deletions cmd/ci-operator/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -831,25 +831,80 @@ func overrideMultiStageParams(o *options) error {
return nil
}

// applyEnvOverrides processes environment variables with override prefixes and applies them to the test configurations.
// It checks for environment variables that start with "MULTISTAGE_PARAM_OVERRIDE_" and applies them to the environment settings of each test.
// multiStageParamOverridePrefix is the legacy prefix used to request a multi-stage parameter override.
const multiStageParamOverridePrefix = "MULTISTAGE_PARAM_OVERRIDE_"

// overridableParamNames returns parameter names, across all Pre/Test/Post steps and
// observers of ms, declared with Overridable: true.
func overridableParamNames(ms *api.MultiStageTestConfigurationLiteral) sets.Set[string] {
names := sets.New[string]()
for _, steps := range [][]api.LiteralTestStep{ms.Pre, ms.Test, ms.Post} {
for _, step := range steps {
for _, param := range step.Environment {
if param.Overridable {
names.Insert(param.Name)
}
}
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
for _, observer := range ms.Observers {
for _, param := range observer.Environment {
if param.Overridable {
names.Insert(param.Name)
}
}
}
return names
}

// applyEnvOverrides applies trigger-time multi-stage parameter overrides from the environment.
// Two forms are supported: the legacy "MULTISTAGE_PARAM_OVERRIDE_<NAME>" prefix (kept for backwards
// compatibility, and takes precedence if both forms are set), and a plain "<NAME>" variable, honored
// only if some step declares that parameter with Overridable: true. Both forms populate ParamOverrides,
// which generateParams (pkg/steps/multi_stage/gen.go) applies only to opted-in parameters.
func applyEnvOverrides(o *options) {
envPairs := make(map[string]string)
for _, envVar := range os.Environ() {
if !strings.HasPrefix(envVar, "MULTISTAGE_PARAM_OVERRIDE_") {
continue
}
parts := strings.SplitN(envVar, "=", 2)
if len(parts) != 2 {
continue
}
key, value := parts[0], parts[1]
for _, test := range o.configSpec.Tests {
if test.MultiStageTestConfigurationLiteral != nil {
if test.MultiStageTestConfigurationLiteral.Environment == nil {
test.MultiStageTestConfigurationLiteral.Environment = make(api.TestEnvironment)
}
test.MultiStageTestConfigurationLiteral.Environment[key] = value
envPairs[parts[0]] = parts[1]
}

for _, test := range o.configSpec.Tests {
ms := test.MultiStageTestConfigurationLiteral
if ms == nil {
continue
}

for key, value := range envPairs {
if !strings.HasPrefix(key, multiStageParamOverridePrefix) {
continue
}
if ms.Environment == nil {
ms.Environment = make(api.TestEnvironment)
}
ms.Environment[key] = value

if ms.ParamOverrides == nil {
ms.ParamOverrides = make(api.TestEnvironment)
}
ms.ParamOverrides[strings.TrimPrefix(key, multiStageParamOverridePrefix)] = value
}

for name := range overridableParamNames(ms) {
if _, alreadySet := ms.ParamOverrides[name]; alreadySet {
continue // prefixed form already set this parameter and takes precedence.
}
value, ok := envPairs[name]
if !ok {
continue
}
if ms.ParamOverrides == nil {
ms.ParamOverrides = make(api.TestEnvironment)
}
ms.ParamOverrides[name] = value
}
}
}
Expand Down
111 changes: 105 additions & 6 deletions cmd/ci-operator/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1337,11 +1337,12 @@ func TestMultiStageParams(t *testing.T) {

func TestApplyEnvOverrides(t *testing.T) {
testCases := []struct {
id string
envVars map[string]string
expectedParams map[string]string
testConfig []api.TestStepConfiguration
expectedErrs []string
id string
envVars map[string]string
expectedParams map[string]string
expectedOverrides map[string]string
testConfig []api.TestStepConfiguration
expectedErrs []string
}{
{
id: "Apply overrides",
Expand All @@ -1354,6 +1355,10 @@ func TestApplyEnvOverrides(t *testing.T) {
"MULTISTAGE_PARAM_OVERRIDE_PARAM1": "VAL1",
"MULTISTAGE_PARAM_OVERRIDE_PARAM2": "VAL2",
},
expectedOverrides: map[string]string{
"PARAM1": "VAL1",
"PARAM2": "VAL2",
},
testConfig: []api.TestStepConfiguration{
{
MultiStageTestConfigurationLiteral: &api.MultiStageTestConfigurationLiteral{
Expand All @@ -1370,7 +1375,8 @@ func TestApplyEnvOverrides(t *testing.T) {
envVars: map[string]string{
"PARAM1": "VAL1",
},
expectedParams: map[string]string{},
expectedParams: map[string]string{},
expectedOverrides: map[string]string{},
testConfig: []api.TestStepConfiguration{
{
MultiStageTestConfigurationLiteral: &api.MultiStageTestConfigurationLiteral{
Expand All @@ -1391,6 +1397,11 @@ func TestApplyEnvOverrides(t *testing.T) {
"MULTISTAGE_PARAM_OVERRIDE_PARAM2": "VAL=2",
"MULTISTAGE_PARAM_OVERRIDE_PARAM3": "VAL2",
},
expectedOverrides: map[string]string{
"PARAM1": "VAL2",
"PARAM2": "VAL=2",
"PARAM3": "VAL2",
},
testConfig: []api.TestStepConfiguration{
{
MultiStageTestConfigurationLiteral: &api.MultiStageTestConfigurationLiteral{
Expand All @@ -1403,6 +1414,87 @@ func TestApplyEnvOverrides(t *testing.T) {
},
},
},
{
id: "plain name honored for an opted-in parameter",
envVars: map[string]string{
"EVAL_MODEL": "gpt-5",
},
expectedParams: map[string]string{},
expectedOverrides: map[string]string{
"EVAL_MODEL": "gpt-5",
},
testConfig: []api.TestStepConfiguration{
{
MultiStageTestConfigurationLiteral: &api.MultiStageTestConfigurationLiteral{
Test: []api.LiteralTestStep{{
As: "step",
Environment: []api.StepParameter{{Name: "EVAL_MODEL", Overridable: true}},
}},
},
},
},
},
{
id: "plain name ignored for a parameter that did not opt in",
envVars: map[string]string{
"EVAL_MODEL": "gpt-5",
},
expectedParams: map[string]string{},
expectedOverrides: map[string]string{},
testConfig: []api.TestStepConfiguration{
{
MultiStageTestConfigurationLiteral: &api.MultiStageTestConfigurationLiteral{
Test: []api.LiteralTestStep{{
As: "step",
Environment: []api.StepParameter{{Name: "EVAL_MODEL"}},
}},
},
},
},
},
{
id: "plain name honored for an opted-in observer parameter",
envVars: map[string]string{
"EVAL_MODEL": "gpt-5",
},
expectedParams: map[string]string{},
expectedOverrides: map[string]string{
"EVAL_MODEL": "gpt-5",
},
testConfig: []api.TestStepConfiguration{
{
MultiStageTestConfigurationLiteral: &api.MultiStageTestConfigurationLiteral{
Observers: []api.Observer{{
Name: "observer",
Environment: []api.StepParameter{{Name: "EVAL_MODEL", Overridable: true}},
}},
},
},
},
},
{
id: "prefixed form takes precedence over the plain form when both are present",
envVars: map[string]string{
"MULTISTAGE_PARAM_OVERRIDE_EVAL_MODEL": "prefixed-value",
"EVAL_MODEL": "plain-value",
},
expectedParams: map[string]string{
"MULTISTAGE_PARAM_OVERRIDE_EVAL_MODEL": "prefixed-value",
},
expectedOverrides: map[string]string{
"EVAL_MODEL": "prefixed-value",
},
testConfig: []api.TestStepConfiguration{
{
MultiStageTestConfigurationLiteral: &api.MultiStageTestConfigurationLiteral{
Test: []api.LiteralTestStep{{
As: "step",
Environment: []api.StepParameter{{Name: "EVAL_MODEL", Overridable: true}},
}},
},
},
},
},
}

for _, tc := range testCases {
Expand All @@ -1423,18 +1515,25 @@ func TestApplyEnvOverrides(t *testing.T) {
applyEnvOverrides(o)

actualParams := make(map[string]string)
actualOverrides := make(map[string]string)

for _, test := range o.configSpec.Tests {
if test.MultiStageTestConfigurationLiteral != nil {
for name, val := range test.MultiStageTestConfigurationLiteral.Environment {
actualParams[name] = val
}
for name, val := range test.MultiStageTestConfigurationLiteral.ParamOverrides {
actualOverrides[name] = val
}
}
}

if diff := cmp.Diff(tc.expectedParams, actualParams); diff != "" {
t.Errorf("actual does not match expected, diff: %s", diff)
}
if diff := cmp.Diff(tc.expectedOverrides, actualOverrides); diff != "" {
t.Errorf("actual overrides do not match expected, diff: %s", diff)
}
})
}
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/api/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1193,6 +1193,11 @@ type StepParameter struct {
Default *string `json:"default,omitempty"`
// Documentation is a textual description of the parameter.
Documentation string `json:"documentation,omitempty"`
// Overridable, if true, allows this parameter to be set via a trigger-time
// environment variable on ci-operator (its own name, or the legacy
// MULTISTAGE_PARAM_OVERRIDE_<NAME> form, which takes precedence).
// Must be explicitly opted into per parameter.
Overridable bool `json:"overridable,omitempty"`
}

// CredentialReference defines a secret to mount into a step and where to mount it.
Expand Down Expand Up @@ -1347,6 +1352,10 @@ type MultiStageTestConfigurationLiteral struct {
Post []LiteralTestStep `json:"post,omitempty"`
// Environment has the values of parameters for the steps.
Environment TestEnvironment `json:"env,omitempty"`
// ParamOverrides holds trigger-time parameter values, populated by
// ci-operator from the environment. Only honored for parameters
// declared with Overridable: true. Not meant to be set in CI config.
ParamOverrides TestEnvironment `json:"param_overrides,omitempty"`
// Dependencies holds override values for dependency parameters.
Dependencies TestDependencies `json:"dependencies,omitempty"`
// DnsConfig for step's Pod.
Expand Down
7 changes: 7 additions & 0 deletions pkg/api/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions pkg/steps/multi_stage/gen.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,13 @@ func (s *multiStageTestStep) generateParams(env []api.StepParameter) []coreapi.E
if v, ok := s.env[env.Name]; ok {
value = v
}
// paramOverrides carries untrusted, trigger-time values. Only honor it if this exact
// parameter opted in, so a same-named parameter on another step never receives it.
if env.Overridable {
if v, ok := s.paramOverrides[env.Name]; ok {
value = v
}
}
ret = append(ret, coreapi.EnvVar{Name: env.Name, Value: value})
}
return ret
Expand Down
Loading