Skip to content

Commit

Permalink
Need validation in both places: resolve and validation
Browse files Browse the repository at this point in the history
Signed-off-by: Hui Kang <[email protected]>
  • Loading branch information
Hui Kang committed May 29, 2021
1 parent b308efc commit e8e4493
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 2 deletions.
5 changes: 3 additions & 2 deletions pkg/apis/rollouts/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,9 @@ func ValidateRolloutSpec(rollout *v1alpha1.Rollout, fldPath *field.Path) field.E
}
}

// WorkloadRef and template can not be set at the same time
if spec.WorkloadRef != nil && !spec.EmptyTemplate() {
if !rollout.Spec.TemplateResolvedFromRef && (spec.WorkloadRef != nil && !spec.EmptyTemplate()) {
// WorkloadRef and template can not be set at the same time for lint plugin
// During reconciliation, TemplateResolvedFromRef is true and will not reach here
allErrs = append(allErrs, field.InternalError(fldPath.Child("template"),
fmt.Errorf("template must be empty for workload reference rollout")))
}
Expand Down
6 changes: 6 additions & 0 deletions rollout/temlateref.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,12 @@ func (r *informerBasedTemplateResolver) Resolve(rollout *v1alpha1.Rollout) error
return nil
}

// When workloadRef is resolved for the first time, TemplateResolvedFromRef = false.
// In this case, template must not be set
if !rollout.Spec.TemplateResolvedFromRef && !rollout.Spec.EmptyTemplate() {
return fmt.Errorf("template must be empty for workload reference rollout")
}

gvk := schema.FromAPIVersionAndKind(rollout.Spec.WorkloadRef.APIVersion, rollout.Spec.WorkloadRef.Kind)

info, ok := infoByGroupKind[gvk.GroupKind()]
Expand Down
49 changes: 49 additions & 0 deletions rollout/temlateref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -406,3 +406,52 @@ func TestRemashalMapFails(t *testing.T) {
err := remarshalMap(nil, struct{}{})
assert.Error(t, err)
}

func TestResolve_WorkloadWithTemplate(t *testing.T) {
rollout := v1alpha1.Rollout{
ObjectMeta: v1.ObjectMeta{
Namespace: "default",
},
Spec: v1alpha1.RolloutSpec{
WorkloadRef: &v1alpha1.ObjectRef{
Name: "my-deployment",
Kind: "Deployment",
APIVersion: "apps/v1",
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"app": "deploy",
},
},
},
},
}

deployment := &appsv1.Deployment{
TypeMeta: metav1.TypeMeta{
APIVersion: appsv1.SchemeGroupVersion.String(),
Kind: "Deployment",
},
ObjectMeta: metav1.ObjectMeta{
Name: "my-deployment",
Namespace: "default",
},
Spec: appsv1.DeploymentSpec{
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"test-label": "test-label-val"}},
},
},
}

discoveryClient := newFakeDiscoClient()
dynamicClient := dynamicfake.NewSimpleDynamicClient(scheme.Scheme, deployment)

resolver, cancel := newResolver(dynamicClient, discoveryClient, fake.NewSimpleClientset())
defer cancel()

err := resolver.Resolve(&rollout)

assert.Error(t, err)
assert.Equal(t, "template must be empty for workload reference rollout", err.Error())
}

0 comments on commit e8e4493

Please sign in to comment.