Skip to content

Commit

Permalink
Merge pull request openshift#1410 from GeoBK/fix/ci-operator/custom-i…
Browse files Browse the repository at this point in the history 
…magestream-validation

Allow custom releases in ci-operator validations
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot authored Nov 16, 2020
2 parents df0a006 + d00f97b commit 3d1a1ae
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 40 deletions.
56 changes: 32 additions & 24 deletions pkg/api/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,11 @@ func (config *ReleaseBuildConfiguration) validate(org, repo string, resolved boo

validationErrors = append(validationErrors, validateReleaseBuildConfiguration(config, org, repo)...)
validationErrors = append(validationErrors, validateBuildRootImageConfiguration("build_root", config.InputConfiguration.BuildRootImage, len(config.Images) > 0))
validationErrors = append(validationErrors, validateTestStepConfiguration("tests", config.Tests, config.ReleaseTagConfiguration, resolved)...)
releases := sets.NewString()
for name := range releases {
releases.Insert(name)
}
validationErrors = append(validationErrors, validateTestStepConfiguration("tests", config.Tests, config.ReleaseTagConfiguration, releases, resolved)...)

// this validation brings together a large amount of data from separate
// parts of the configuration, so it's written as a standalone method
Expand Down Expand Up @@ -340,7 +344,7 @@ func validateOperator(fieldRoot string, input *OperatorStepConfiguration, linkFo
return validationErrors
}

func validateTestStepConfiguration(fieldRoot string, input []TestStepConfiguration, release *ReleaseTagConfiguration, resolved bool) []error {
func validateTestStepConfiguration(fieldRoot string, input []TestStepConfiguration, release *ReleaseTagConfiguration, releases sets.String, resolved bool) []error {
var validationErrors []error

// check for test.As duplicates
Expand Down Expand Up @@ -396,7 +400,7 @@ func validateTestStepConfiguration(fieldRoot string, input []TestStepConfigurati
}
}

validationErrors = append(validationErrors, validateTestConfigurationType(fieldRootN, test, release, resolved)...)
validationErrors = append(validationErrors, validateTestConfigurationType(fieldRootN, test, release, releases, resolved)...)
}
return validationErrors
}
Expand Down Expand Up @@ -479,7 +483,7 @@ func searchForTestDuplicates(tests []TestStepConfiguration) []error {
return nil
}

func validateTestConfigurationType(fieldRoot string, test TestStepConfiguration, release *ReleaseTagConfiguration, resolved bool) []error {
func validateTestConfigurationType(fieldRoot string, test TestStepConfiguration, release *ReleaseTagConfiguration, releases sets.String, resolved bool) []error {
var validationErrors []error
typeCount := 0
if testConfig := test.ContainerTestConfiguration; testConfig != nil {
Expand Down Expand Up @@ -543,9 +547,9 @@ func validateTestConfigurationType(fieldRoot string, test TestStepConfiguration,
validationErrors = append(validationErrors, validateClusterProfile(fieldRoot, testConfig.ClusterProfile)...)
}
seen := sets.NewString()
validationErrors = append(validationErrors, validateTestStepsPre(fmt.Sprintf("%s.Pre", fieldRoot), testConfig.Pre, seen, testConfig.Environment)...)
validationErrors = append(validationErrors, validateTestStepsTest(fmt.Sprintf("%s.Test", fieldRoot), testConfig.Test, seen, testConfig.Environment)...)
validationErrors = append(validationErrors, validateTestStepsPost(fmt.Sprintf("%s.Post", fieldRoot), testConfig.Post, seen, testConfig.Environment)...)
validationErrors = append(validationErrors, validateTestStepsPre(fmt.Sprintf("%s.Pre", fieldRoot), testConfig.Pre, seen, testConfig.Environment, releases)...)
validationErrors = append(validationErrors, validateTestStepsTest(fmt.Sprintf("%s.Test", fieldRoot), testConfig.Test, seen, testConfig.Environment, releases)...)
validationErrors = append(validationErrors, validateTestStepsPost(fmt.Sprintf("%s.Post", fieldRoot), testConfig.Post, seen, testConfig.Environment, releases)...)
}
if testConfig := test.MultiStageTestConfigurationLiteral; testConfig != nil {
typeCount++
Expand All @@ -555,15 +559,15 @@ func validateTestConfigurationType(fieldRoot string, test TestStepConfiguration,
seen := sets.NewString()
for i, s := range testConfig.Pre {
fieldRootI := fmt.Sprintf("%s.Pre[%d]", fieldRoot, i)
validationErrors = append(validationErrors, validateLiteralTestStepPre(fieldRootI, s, seen, testConfig.Environment)...)
validationErrors = append(validationErrors, validateLiteralTestStepPre(fieldRootI, s, seen, testConfig.Environment, releases)...)
}
for i, s := range testConfig.Test {
fieldRootI := fmt.Sprintf("%s.Test[%d]", fieldRoot, i)
validationErrors = append(validationErrors, validateLiteralTestStepTest(fieldRootI, s, seen, testConfig.Environment)...)
validationErrors = append(validationErrors, validateLiteralTestStepTest(fieldRootI, s, seen, testConfig.Environment, releases)...)
}
for i, s := range testConfig.Post {
fieldRootI := fmt.Sprintf("%s.Post[%d]", fieldRoot, i)
validationErrors = append(validationErrors, validateLiteralTestStepPost(fieldRootI, s, seen, testConfig.Environment)...)
validationErrors = append(validationErrors, validateLiteralTestStepPost(fieldRootI, s, seen, testConfig.Environment, releases)...)
}
}
if typeCount == 0 {
Expand All @@ -579,34 +583,34 @@ func validateTestConfigurationType(fieldRoot string, test TestStepConfiguration,
return validationErrors
}

func validateTestStepsPre(fieldRoot string, steps []TestStep, seen sets.String, env TestEnvironment) (ret []error) {
func validateTestStepsPre(fieldRoot string, steps []TestStep, seen sets.String, env TestEnvironment, releases sets.String) (ret []error) {
for i, s := range steps {
fieldRootI := fmt.Sprintf("%s[%d]", fieldRoot, i)
ret = validateTestStep(fieldRootI, s, seen)
if s.LiteralTestStep != nil {
ret = append(ret, validateLiteralTestStepPre(fieldRootI, *s.LiteralTestStep, seen, env)...)
ret = append(ret, validateLiteralTestStepPre(fieldRootI, *s.LiteralTestStep, seen, env, releases)...)
}
}
return
}

func validateTestStepsTest(fieldRoot string, steps []TestStep, seen sets.String, env TestEnvironment) (ret []error) {
func validateTestStepsTest(fieldRoot string, steps []TestStep, seen sets.String, env TestEnvironment, releases sets.String) (ret []error) {
for i, s := range steps {
fieldRootI := fmt.Sprintf("%s[%d]", fieldRoot, i)
ret = validateTestStep(fieldRootI, s, seen)
if s.LiteralTestStep != nil {
ret = append(ret, validateLiteralTestStepTest(fieldRootI, *s.LiteralTestStep, seen, env)...)
ret = append(ret, validateLiteralTestStepTest(fieldRootI, *s.LiteralTestStep, seen, env, releases)...)
}
}
return
}

func validateTestStepsPost(fieldRoot string, steps []TestStep, seen sets.String, env TestEnvironment) (ret []error) {
func validateTestStepsPost(fieldRoot string, steps []TestStep, seen sets.String, env TestEnvironment, releases sets.String) (ret []error) {
for i, s := range steps {
fieldRootI := fmt.Sprintf("%s[%d]", fieldRoot, i)
ret = validateTestStep(fieldRootI, s, seen)
if s.LiteralTestStep != nil {
ret = append(ret, validateLiteralTestStepPost(fieldRootI, *s.LiteralTestStep, seen, env)...)
ret = append(ret, validateLiteralTestStepPost(fieldRootI, *s.LiteralTestStep, seen, env, releases)...)
}
}
return
Expand Down Expand Up @@ -644,7 +648,7 @@ func validateTestStep(fieldRootI string, step TestStep, seen sets.String) (ret [
return
}

func validateLiteralTestStepCommon(fieldRoot string, step LiteralTestStep, seen sets.String, env TestEnvironment) (ret []error) {
func validateLiteralTestStepCommon(fieldRoot string, step LiteralTestStep, seen sets.String, env TestEnvironment, releases sets.String) (ret []error) {
if len(step.As) == 0 {
ret = append(ret, fmt.Errorf("%s: `as` is required", fieldRoot))
} else if seen.Has(step.As) {
Expand Down Expand Up @@ -678,7 +682,11 @@ func validateLiteralTestStepCommon(fieldRoot string, step LiteralTestStep, seen
switch obj {
case PipelineImageStream, ReleaseStreamFor(LatestReleaseName), ReleaseStreamFor(InitialReleaseName), ReleaseImageStream:
default:
ret = append(ret, fmt.Errorf("%s.from: unknown imagestream '%s'", fieldRoot, imageParts[0]))
releaseName := ReleaseNameFrom(obj)
if !releases.Has(releaseName) {
ret = append(ret, fmt.Errorf("%s.from: unknown imagestream '%s'", fieldRoot, imageParts[0]))
}

}
}
}
Expand All @@ -695,21 +703,21 @@ func validateLiteralTestStepCommon(fieldRoot string, step LiteralTestStep, seen
return
}

func validateLiteralTestStepPre(fieldRoot string, step LiteralTestStep, seen sets.String, env TestEnvironment) (ret []error) {
ret = validateLiteralTestStepCommon(fieldRoot, step, seen, env)
func validateLiteralTestStepPre(fieldRoot string, step LiteralTestStep, seen sets.String, env TestEnvironment, releases sets.String) (ret []error) {
ret = validateLiteralTestStepCommon(fieldRoot, step, seen, env, releases)
if step.OptionalOnSuccess != nil {
ret = append(ret, fmt.Errorf("%s: `optional_on_success` is only allowed for Post steps", fieldRoot))
}
return
}

func validateLiteralTestStepTest(fieldRoot string, step LiteralTestStep, seen sets.String, env TestEnvironment) (ret []error) {
ret = validateLiteralTestStepPre(fieldRoot, step, seen, env)
func validateLiteralTestStepTest(fieldRoot string, step LiteralTestStep, seen sets.String, env TestEnvironment, releases sets.String) (ret []error) {
ret = validateLiteralTestStepPre(fieldRoot, step, seen, env, releases)
return
}

func validateLiteralTestStepPost(fieldRoot string, step LiteralTestStep, seen sets.String, env TestEnvironment) (ret []error) {
ret = validateLiteralTestStepCommon(fieldRoot, step, seen, env)
func validateLiteralTestStepPost(fieldRoot string, step LiteralTestStep, seen sets.String, env TestEnvironment, releases sets.String) (ret []error) {
ret = validateLiteralTestStepCommon(fieldRoot, step, seen, env, releases)
return
}

Expand Down
57 changes: 41 additions & 16 deletions pkg/api/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ func TestValidateTests(t *testing.T) {
for _, tc := range []struct {
id string
release *ReleaseTagConfiguration
releases sets.String
tests []TestStepConfiguration
resolved bool
expectedValid bool
Expand Down Expand Up @@ -388,7 +389,7 @@ func TestValidateTests(t *testing.T) {
},
} {
t.Run(tc.id, func(t *testing.T) {
if errs := validateTestStepConfiguration("tests", tc.tests, tc.release, tc.resolved); len(errs) > 0 && tc.expectedValid {
if errs := validateTestStepConfiguration("tests", tc.tests, tc.release, tc.releases, tc.resolved); len(errs) > 0 && tc.expectedValid {
t.Errorf("expected to be valid, got: %v", errs)
} else if !tc.expectedValid && len(errs) == 0 {
t.Error("expected to be invalid, but returned valid")
Expand Down Expand Up @@ -528,10 +529,11 @@ func TestValidateTestSteps(t *testing.T) {
asReference := "as"
yes := true
for _, tc := range []struct {
name string
steps []TestStep
seen sets.String
errs []error
name string
steps []TestStep
seen sets.String
errs []error
releases sets.String
}{{
name: "valid step",
steps: []TestStep{{
Expand Down Expand Up @@ -687,6 +689,27 @@ func TestValidateTestSteps(t *testing.T) {
Resources: resources},
}},
errs: []error{errors.New("test[0].from: unknown imagestream 'no-such-imagestream'")},
}, {
name: "custom imagestream",
steps: []TestStep{{
LiteralTestStep: &LiteralTestStep{
As: "as",
From: "stable-previous:base",
Commands: "commands",
Resources: resources},
}},
releases: sets.NewString("previous"),
}, {
name: "invalid image 4",
steps: []TestStep{{
LiteralTestStep: &LiteralTestStep{
As: "as",
From: "stable-nonexistent:base",
Commands: "commands",
Resources: resources},
}},
releases: sets.NewString("previous"),
errs: []error{errors.New("test[0].from: unknown imagestream 'stable-nonexistent'")},
}, {
name: "no commands",
steps: []TestStep{{
Expand Down Expand Up @@ -759,7 +782,7 @@ func TestValidateTestSteps(t *testing.T) {
if seen == nil {
seen = sets.NewString()
}
ret := validateTestStepsTest("test", tc.steps, seen, nil)
ret := validateTestStepsTest("test", tc.steps, seen, nil, tc.releases)
if !errListMessagesEqual(ret, tc.errs) {
t.Fatal(diff.ObjectReflectDiff(ret, tc.errs))
}
Expand All @@ -774,10 +797,11 @@ func TestValidatePostSteps(t *testing.T) {
}
yes := true
for _, tc := range []struct {
name string
steps []TestStep
seen sets.String
errs []error
name string
steps []TestStep
seen sets.String
errs []error
releases sets.String
}{{
name: "Valid Post steps",

Expand All @@ -795,7 +819,7 @@ func TestValidatePostSteps(t *testing.T) {
if seen == nil {
seen = sets.NewString()
}
ret := validateTestStepsPost("test", tc.steps, seen, nil)
ret := validateTestStepsPost("test", tc.steps, seen, nil, tc.releases)
if !errListMessagesEqual(ret, tc.errs) {
t.Fatal(diff.ObjectReflectDiff(ret, tc.errs))
}
Expand All @@ -806,10 +830,11 @@ func TestValidatePostSteps(t *testing.T) {
func TestValidateParameters(t *testing.T) {
defaultStr := "default"
for _, tc := range []struct {
name string
params []StepParameter
env TestEnvironment
err []error
name string
params []StepParameter
env TestEnvironment
err []error
releases sets.String
}{{
name: "no parameters",
}, {
Expand All @@ -835,7 +860,7 @@ func TestValidateParameters(t *testing.T) {
Limits: ResourceList{"memory": "1m"},
},
Environment: tc.params,
}, sets.NewString(), tc.env)
}, sets.NewString(), tc.env, tc.releases)
if diff := diff.ObjectReflectDiff(err, tc.err); diff != "<no diffs>" {
t.Errorf("incorrect error: %s", diff)
}
Expand Down

0 comments on commit 3d1a1ae

Please sign in to comment.