diff --git a/pkg/reconciler/build/build_test.go b/pkg/reconciler/build/build_test.go index 06070d4ccb..83d9308f04 100644 --- a/pkg/reconciler/build/build_test.go +++ b/pkg/reconciler/build/build_test.go @@ -312,7 +312,7 @@ var _ = Describe("Reconcile Build", func() { Context("when spec strategy kind is unknown", func() { JustBeforeEach(func() { - buildStrategyKind := build.BuildStrategyKind("abc") + buildStrategyKind := buildapi.BuildStrategyKind("abc") buildStrategyName = "xyz" buildName = "build-name" buildSample = ctl.BuildWithNilBuildStrategyKind(buildName, namespace, buildStrategyName) @@ -321,10 +321,10 @@ var _ = Describe("Reconcile Build", func() { It("should fail validation and update the status to indicate that the strategy kind is unknown", func() { statusWriter.UpdateCalls(func(ctx context.Context, o crc.Object, sruo ...crc.SubResourceUpdateOption) error { - Expect(o).To(BeAssignableToTypeOf(&build.Build{})) - b := o.(*build.Build) + Expect(o).To(BeAssignableToTypeOf(&buildapi.Build{})) + b := o.(*buildapi.Build) Expect(b.Status.Reason).ToNot(BeNil()) - Expect(*b.Status.Reason).To(Equal(build.UnknownBuildStrategyKind)) + Expect(*b.Status.Reason).To(Equal(buildapi.UnknownBuildStrategyKind)) return nil }) diff --git a/pkg/validate/strategies.go b/pkg/validate/strategies.go index a4fb505c67..3e03325a35 100644 --- a/pkg/validate/strategies.go +++ b/pkg/validate/strategies.go @@ -32,63 +32,63 @@ func NewStrategies(client client.Client, build *buildapi.Build) *Strategy { // that the referenced strategy exists. This applies to both // namespaced or cluster scoped strategies func (s Strategy) ValidatePath(ctx context.Context) error { - var ( - builderStrategy buildapi.BuilderStrategy - strategyExists bool - err error - ) - - if s.Build.Spec.Strategy.Kind != nil { - switch *s.Build.Spec.Strategy.Kind { - case buildapi.NamespacedBuildStrategyKind: - buildStrategy := &buildapi.BuildStrategy{} - strategyExists, err = s.validateBuildStrategy(ctx, s.Build.Spec.Strategy.Name, buildStrategy) - builderStrategy = buildStrategy - case buildapi.ClusterBuildStrategyKind: - clusterBuildStrategy := &buildapi.ClusterBuildStrategy{} - strategyExists, err = s.validateClusterBuildStrategy(ctx, s.Build.Spec.Strategy.Name, clusterBuildStrategy) - builderStrategy = clusterBuildStrategy - default: - return fmt.Errorf("unknown strategy kind: %v", *s.Build.Spec.Strategy.Kind) - } - } else { - ctxlog.Info(ctx, "buildStrategy kind is nil, use default NamespacedBuildStrategyKind", namespace, s.Build.Namespace, name, s.Build.Name) - buildStrategy := &buildapi.BuildStrategy{} - strategyExists, err = s.validateBuildStrategy(ctx, s.Build.Spec.Strategy.Name, buildStrategy) - builderStrategy = buildStrategy - } - - if err != nil { - return err + switch s.kind(ctx) { + case buildapi.NamespacedBuildStrategyKind: + return s.validateBuildStrategy(ctx, s.Build.Spec.Strategy.Name) + + case buildapi.ClusterBuildStrategyKind: + return s.validateClusterBuildStrategy(ctx, s.Build.Spec.Strategy.Name) + + default: + s.Build.Status.Reason = buildapi.BuildReasonPtr(buildapi.UnknownBuildStrategyKind) + s.Build.Status.Message = pointer.String(fmt.Sprintf("unknown strategy kind %s used, must be one of %s, or %s", + *s.Build.Spec.Strategy.Kind, + buildapi.NamespacedBuildStrategyKind, + buildapi.ClusterBuildStrategyKind)) + return nil } +} - if strategyExists { - s.validateBuildParams(builderStrategy.GetParameters()) - s.validateBuildVolumes(builderStrategy.GetVolumes()) +func (s Strategy) kind(ctx context.Context) buildapi.BuildStrategyKind { + if s.Build.Spec.Strategy.Kind == nil { + ctxlog.Info(ctx, "buildStrategy kind is nil, use default NamespacedBuildStrategyKind", namespace, s.Build.Namespace, name, s.Build.Name) + return buildapi.NamespacedBuildStrategyKind } - return nil + return *s.Build.Spec.Strategy.Kind } -func (s Strategy) validateBuildStrategy(ctx context.Context, strategyName string, buildStrategy *buildapi.BuildStrategy) (bool, error) { - if err := s.Client.Get(ctx, types.NamespacedName{Name: strategyName, Namespace: s.Build.Namespace}, buildStrategy); err != nil && !apierrors.IsNotFound(err) { - return false, err - } else if apierrors.IsNotFound(err) { +func (s Strategy) validateBuildStrategy(ctx context.Context, strategyName string) error { + buildStrategy := &buildapi.BuildStrategy{} + err := s.Client.Get(ctx, types.NamespacedName{Name: strategyName, Namespace: s.Build.Namespace}, buildStrategy) + if err == nil { + s.validateBuildParams(buildStrategy.GetParameters()) + s.validateBuildVolumes(buildStrategy.GetVolumes()) + return nil + } + + if apierrors.IsNotFound(err) { s.Build.Status.Reason = buildapi.BuildReasonPtr(buildapi.BuildStrategyNotFound) - s.Build.Status.Message = pointer.String(fmt.Sprintf("buildStrategy %s does not exist in namespace %s", s.Build.Spec.Strategy.Name, s.Build.Namespace)) - return false, nil + s.Build.Status.Message = pointer.String(fmt.Sprintf("buildStrategy %s does not exist in namespace %s", strategyName, s.Build.Namespace)) + return nil } return err } -func (s Strategy) validateClusterBuildStrategy(ctx context.Context, strategyName string, clusterBuildStrategy *buildapi.ClusterBuildStrategy) (bool, error) { - if err := s.Client.Get(ctx, types.NamespacedName{Name: strategyName}, clusterBuildStrategy); err != nil && !apierrors.IsNotFound(err) { - return false, err - } else if apierrors.IsNotFound(err) { +func (s Strategy) validateClusterBuildStrategy(ctx context.Context, strategyName string) error { + clusterBuildStrategy := &buildapi.ClusterBuildStrategy{} + err := s.Client.Get(ctx, types.NamespacedName{Name: strategyName}, clusterBuildStrategy) + if err == nil { + s.validateBuildParams(clusterBuildStrategy.GetParameters()) + s.validateBuildVolumes(clusterBuildStrategy.GetVolumes()) + return nil + } + + if apierrors.IsNotFound(err) { s.Build.Status.Reason = buildapi.BuildReasonPtr(buildapi.ClusterBuildStrategyNotFound) - s.Build.Status.Message = pointer.String(fmt.Sprintf("clusterBuildStrategy %s does not exist", s.Build.Spec.Strategy.Name)) - return false, nil + s.Build.Status.Message = pointer.String(fmt.Sprintf("clusterBuildStrategy %s does not exist", strategyName)) + return nil } return err