Removed Optional field, faster volume merging

Optional field has been declared unnecessary. It has been removed
in this commit.
Previous volume merging algorithm has been implemented with
quadratic complexity, while linear is possible. Been reworked
to linear.

deploy/crds/shipwright.io_buildstrategies.yaml

@@ -1302,9 +1302,6 @@ spec:
                     name:
                       description: Name of the Build Volume
                       type: string
-                    optional:
-                      description: Whether this Volume is optional or not
-                      type: boolean
                     overridable:
                       description: Indicates that this Volume can be overrided in
                         a Build or BuildRun

deploy/crds/shipwright.io_clusterbuildstrategies.yaml

@@ -1302,9 +1302,6 @@ spec:
                     name:
                       description: Name of the Build Volume
                       type: string
-                    optional:
-                      description: Whether this Volume is optional or not
-                      type: boolean
                     overridable:
                       description: Indicates that this Volume can be overrided in
                         a Build or BuildRun

pkg/apis/build/v1alpha1/buildstrategy.go

@@ -79,10 +79,6 @@ type BuildStrategyVolume struct {
 	// +optional
 	Description *string `json:"description,omitempty"`
 
-	// Whether this Volume is optional or not
-	// +optional
-	Optional *bool `json:"optional,omitempty"`
-
 	// Indicates that this Volume can be overrided
 	// in a Build or BuildRun
 	// +optional

pkg/volumes/volumes.go

@@ -51,11 +51,8 @@ func TaskSpecVolumes(
 	}
 
 	for _, v := range volumes {
-		volumeMountExists := false
 		for _, volumeMount := range existingVolumeMounts {
 			if volumeMount.Name == v.Name {
-				volumeMountExists = true
-
 				// In case volume mount is not read only and
 				// volume type is either secret or config map,
 				// build should not be run, because this situation may lead
@@ -69,10 +66,6 @@ func TaskSpecVolumes(
 			}
 		}
 
-		if !volumeMountExists && v.Optional != nil && !*v.Optional {
-			return nil, fmt.Errorf("Volume %q is not optional and does not have corresponding Volume Mount", v.Name)
-		}
-
 		taskRunVolume := corev1.Volume{
 			Name:         v.Name,
 			VolumeSource: v.VolumeSource,
@@ -95,33 +88,37 @@ func MergeBuildVolumes(into []buildv1alpha1.BuildStrategyVolume, new []buildv1al
 		return into, nil
 	}
 
-	var result []buildv1alpha1.BuildStrategyVolume
+	mergeMap := make(map[string]buildv1alpha1.BuildStrategyVolume)
 	var errors []error
 
 	for _, vol := range into {
-		result = append(result, *vol.DeepCopy())
+		if _, ok := mergeMap[vol.Name]; ok {
+			return nil, fmt.Errorf("Build Strategy %q is listed more than once", vol.Name)
+		}
+		mergeMap[vol.Name] = *vol.DeepCopy()
 	}
 
 	for _, merger := range new {
-		found := false
-		for i, original := range into {
-			if original.Name == merger.Name {
-				// in case overridable is nil OR false (default is considered false)
-				// then return error, otherwise means it is not nil AND true
-				if original.Overridable == nil || !*original.Overridable {
-					errors = append(errors, fmt.Errorf("Cannot override BuildVolume %q", original.Name))
-					continue
-				}
-
-				found = true
-				result[i].VolumeSource = merger.VolumeSource
-				break
-			}
+		original, ok := mergeMap[merger.Name]
+		if !ok {
+			errors = append(errors, fmt.Errorf("Build Volume %q is not found in the BuildStrategy", merger.Name))
+			continue
 		}
 
-		if !found {
-			errors = append(errors, fmt.Errorf("BuildVolume %q is not found in the BuildStrategy", merger.Name))
+		// in case overridable is nil OR false (default is considered false)
+		// then return error, otherwise means it is not nil AND true
+		if original.Overridable == nil || !*original.Overridable {
+			errors = append(errors, fmt.Errorf("Cannot override BuildVolume %q", original.Name))
+			continue
 		}
+
+		original.VolumeSource = merger.VolumeSource
+		mergeMap[merger.Name] = original
+	}
+
+	result := make([]buildv1alpha1.BuildStrategyVolume, 0, len(mergeMap))
+	for _, v := range mergeMap {
+		result = append(result, v)
 	}
 
 	return result, kerrors.NewAggregate(errors)