From 9228b2b19ea7f59661ed8f46c7af3afb7318b24a Mon Sep 17 00:00:00 2001 From: Vu Dinh Date: Wed, 29 Jul 2020 12:27:41 -0400 Subject: [PATCH] fix(validation): Remove default channel validation from bundle lib It is not possible to determine if the default channel value is valid or not given we can "infer" default channel by using an emptry string. Also, author can add a default channel that is not listed in the channel list in some cases. As a result, the default channel is subjective and there is no methodology to validate it. Therefore, removing this validation will avoid any false positve. Signed-off-by: Vu Dinh --- pkg/lib/bundle/generate.go | 40 ++------------------------------- pkg/lib/bundle/generate_test.go | 37 ------------------------------ pkg/lib/bundle/validate.go | 8 +++---- 3 files changed, 5 insertions(+), 80 deletions(-) diff --git a/pkg/lib/bundle/generate.go b/pkg/lib/bundle/generate.go index 0e72c32bf..ed56d18a4 100644 --- a/pkg/lib/bundle/generate.go +++ b/pkg/lib/bundle/generate.go @@ -293,32 +293,6 @@ func ValidateAnnotations(existing, expected []byte) error { return utilerrors.NewAggregate(errs) } -// ValidateChannelDefault validates provided default channel to ensure it exists in -// provided channel list. -func ValidateChannelDefault(channels, channelDefault string) (string, error) { - var chanDefault string - var chanErr error - channelList := strings.Split(channels, ",") - - if containsString(channelList, "") { - return chanDefault, fmt.Errorf("invalid channels are provided: %s", channels) - } - - if channelDefault != "" { - for _, channel := range channelList { - if channel == channelDefault { - chanDefault = channelDefault - break - } - } - if chanDefault == "" { - chanDefault = channelList[0] - chanErr = fmt.Errorf(`The channel list "%s" doesn't contain channelDefault "%s"`, channels, channelDefault) - } - } - return chanDefault, chanErr -} - // GenerateAnnotations builds annotations.yaml with mediatype, manifests & // metadata directories in bundle image, package name, channels and default // channels information. @@ -334,12 +308,7 @@ func GenerateAnnotations(mediaType, manifests, metadata, packageName, channels, }, } - chanDefault, err := ValidateChannelDefault(channels, channelDefault) - if err != nil { - return nil, err - } - - annotations.Annotations[ChannelDefaultLabel] = chanDefault + annotations.Annotations[ChannelDefaultLabel] = channelDefault afile, err := yaml.Marshal(annotations) if err != nil { @@ -355,11 +324,6 @@ func GenerateAnnotations(mediaType, manifests, metadata, packageName, channels, func GenerateDockerfile(mediaType, manifests, metadata, copyManifestDir, copyMetadataDir, workingDir, packageName, channels, channelDefault string) ([]byte, error) { var fileContent string - chanDefault, err := ValidateChannelDefault(channels, channelDefault) - if err != nil { - return nil, err - } - relativeManifestDirectory, err := filepath.Rel(workingDir, copyManifestDir) if err != nil { return nil, err @@ -379,7 +343,7 @@ func GenerateDockerfile(mediaType, manifests, metadata, copyManifestDir, copyMet fileContent += fmt.Sprintf("LABEL %s=%s\n", MetadataLabel, metadata) fileContent += fmt.Sprintf("LABEL %s=%s\n", PackageLabel, packageName) fileContent += fmt.Sprintf("LABEL %s=%s\n", ChannelsLabel, channels) - fileContent += fmt.Sprintf("LABEL %s=%s\n\n", ChannelDefaultLabel, chanDefault) + fileContent += fmt.Sprintf("LABEL %s=%s\n\n", ChannelDefaultLabel, channelDefault) // CONTENT fileContent += fmt.Sprintf("COPY %s %s\n", relativeManifestDirectory, "/manifests/") diff --git a/pkg/lib/bundle/generate_test.go b/pkg/lib/bundle/generate_test.go index c6716a3ae..29270cbba 100644 --- a/pkg/lib/bundle/generate_test.go +++ b/pkg/lib/bundle/generate_test.go @@ -49,43 +49,6 @@ func TestGetMediaType(t *testing.T) { } } -func TestValidateChannelDefault(t *testing.T) { - tests := []struct { - channels string - channelDefault string - result string - errorMsg string - }{ - { - "test5,test6", - "", - "", - "", - }, - { - "test5,test6", - "test7", - "test5", - `The channel list "test5,test6" doesn't contain channelDefault "test7"`, - }, - { - ",", - "", - "", - `invalid channels are provided: ,`, - }, - } - - for _, item := range tests { - output, err := ValidateChannelDefault(item.channels, item.channelDefault) - if item.errorMsg == "" { - require.Equal(t, item.result, output) - } else { - require.Equal(t, item.errorMsg, err.Error()) - } - } -} - func TestValidateAnnotations(t *testing.T) { tests := []struct { existing []byte diff --git a/pkg/lib/bundle/validate.go b/pkg/lib/bundle/validate.go index 2aa580d41..40da1c04e 100644 --- a/pkg/lib/bundle/validate.go +++ b/pkg/lib/bundle/validate.go @@ -201,20 +201,18 @@ func validateAnnotations(mediaType string, fileAnnotations *AnnotationMetadata) aErr := fmt.Errorf("Expecting annotation %q to have value %q instead of %q", label, MetadataDir, val) validationErrors = append(validationErrors, aErr) } - case ChannelsLabel, ChannelDefaultLabel: + case ChannelsLabel: if val == "" { aErr := fmt.Errorf("Expecting annotation %q to have non-empty value", label) validationErrors = append(validationErrors, aErr) } else { annotations[label] = val } + case ChannelDefaultLabel: + annotations[label] = val } } - _, err := ValidateChannelDefault(annotations[ChannelsLabel], annotations[ChannelDefaultLabel]) - if err != nil { - validationErrors = append(validationErrors, err) - } return validationErrors }