Skip to content

Commit

Permalink
chore: should report error when resolving partial required objects (#…
Browse files Browse the repository at this point in the history 
…7232)

cherry picked from commit 5543ebb
  • Loading branch information
@leon-inf
leon-inf authored May 6, 2024
1 parent ad18393 commit 251f11f
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 12 deletions.
17 changes: 12 additions & 5 deletions pkg/controller/component/vars.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -872,11 +872,11 @@ func resolveClusterObjectVars(kind string, objRef appsv1alpha1.ClusterObjectRefe
if err != nil {
return nil, nil, fmt.Errorf("resolving vars from %s object %s error: %s", kind, objRef.Name, err.Error())
}
if len(objs) == 0 || isAllNil(objs) {
if objOptional() {
return nil, nil, nil
}
return nil, nil, fmt.Errorf("%s object %s is not found when resolving vars", kind, objRef.Name)
switch {
case objOptional() && isAllNil(objs):
return nil, nil, nil
case !objOptional() && (len(objs) == 0 || isHasNil(objs)):
return nil, nil, fmt.Errorf("has %s object %s not found when resolving vars", kind, objRef.Name)
}

vars1, vars2 := make(map[string]*corev1.EnvVar), make(map[string]*corev1.EnvVar)
Expand Down Expand Up @@ -1040,6 +1040,13 @@ func isAllNil(objs map[string]any) bool {
return generics.CountFunc(maps.Values(objs), isNil) == len(objs)
}

func isHasNil(objs map[string]any) bool {
isNil := func(o any) bool {
return o == nil
}
return generics.CountFunc(maps.Values(objs), isNil) > 0
}

func isAllVarsNil(vars map[string]*corev1.EnvVar) bool {
isNil := func(v *corev1.EnvVar) bool {
return v == nil
Expand Down
118 changes: 111 additions & 7 deletions pkg/controller/component/vars_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -940,6 +940,7 @@ var _ = Describe("vars", func() {
compName3 = synthesizedComp.Name + "-other-not-exist"
svcName1 = constant.GenerateComponentServiceName(synthesizedComp.ClusterName, compName1, "service")
svcName2 = constant.GenerateComponentServiceName(synthesizedComp.ClusterName, compName2, "service")
svcName3 = constant.GenerateComponentServiceName(synthesizedComp.ClusterName, compName3, "service")
credentialSecretName1 = constant.GenerateAccountSecretName(synthesizedComp.ClusterName, compName1, "credential")
credentialSecretName2 = constant.GenerateAccountSecretName(synthesizedComp.ClusterName, compName2, "credential")

Expand All @@ -952,8 +953,9 @@ var _ = Describe("vars", func() {
compCredentialVarName1 = compVarName(compName1, "credential-username")
compCredentialVarName2 = compVarName(compName2, "credential-username")

combinedSvcVarValue = fmt.Sprintf("%s:%s,%s:%s", compName1, svcName1, compName2, svcName2)
combinedSvcVarValueWithComp3 = fmt.Sprintf("%s:%s,%s:%s,%s:", compName1, svcName1, compName2, svcName2, compName3)
combinedSvcVarValue = fmt.Sprintf("%s:%s,%s:%s", compName1, svcName1, compName2, svcName2)
combinedSvcVarValueWithComp3KeyOnly = fmt.Sprintf("%s:%s,%s:%s,%s:", compName1, svcName1, compName2, svcName2, compName3)
combinedSvcVarValueWithComp3 = fmt.Sprintf("%s:%s,%s:%s,%s:%s", compName1, svcName1, compName2, svcName2, compName3, svcName3)

newVarSuffix = "suffix"
newCombinedSvcVarName = fmt.Sprintf("%s_%s", "service-host", newVarSuffix)
Expand Down Expand Up @@ -1222,7 +1224,7 @@ var _ = Describe("vars", func() {
Expect(err).ShouldNot(Succeed())
Expect(err.Error()).Should(ContainSubstring("combined strategy doesn't support vars with valueFrom values"))

By("individual - partial objects")
By("individual - optional partial objects")
synthesizedComp.Comp2CompDefs = map[string]string{
compName1: synthesizedComp.CompDefName,
compName2: synthesizedComp.CompDefName,
Expand All @@ -1236,7 +1238,7 @@ var _ = Describe("vars", func() {
ClusterObjectReference: appsv1alpha1.ClusterObjectReference{
CompDef: synthesizedComp.CompDefName,
Name: "service",
Optional: required(),
Optional: optional(), // optional
MultipleClusterObjectOption: &appsv1alpha1.MultipleClusterObjectOption{
Strategy: appsv1alpha1.MultipleClusterObjectStrategyIndividual,
},
Expand All @@ -1261,7 +1263,7 @@ var _ = Describe("vars", func() {
// the new var for comp3 will still be created, but its values will be empty.
checkEnvVarWithValue(envVars, compSvcVarName3, "")

By("combined - partial objects")
By("individual - required partial objects")
vars = []appsv1alpha1.EnvVar{
{
Name: "service-host",
Expand All @@ -1270,7 +1272,58 @@ var _ = Describe("vars", func() {
ClusterObjectReference: appsv1alpha1.ClusterObjectReference{
CompDef: synthesizedComp.CompDefName,
Name: "service",
Optional: required(),
Optional: required(), // required
MultipleClusterObjectOption: &appsv1alpha1.MultipleClusterObjectOption{
Strategy: appsv1alpha1.MultipleClusterObjectStrategyIndividual,
},
},
ServiceVars: appsv1alpha1.ServiceVars{
Host: &appsv1alpha1.VarRequired,
},
},
},
},
}
_, _, err = ResolveTemplateNEnvVars(testCtx.Ctx, reader, synthesizedComp, nil, vars)
Expect(err).ShouldNot(BeNil())
Expect(err.Error()).Should(ContainSubstring("not found when resolving vars"))
// create service for comp3
reader.objs = append(reader.objs, &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Namespace: testCtx.DefaultNamespace,
Name: svcName3,
},
Spec: corev1.ServiceSpec{
Ports: []corev1.ServicePort{
{
Port: int32(3306),
},
},
},
})
templateVars, envVars, err = ResolveTemplateNEnvVars(testCtx.Ctx, reader, synthesizedComp, nil, vars)
Expect(err).Should(BeNil())
Expect(templateVars).Should(HaveKeyWithValue("service-host", ""))
Expect(templateVars).Should(HaveKeyWithValue(compSvcVarName1, svcName1))
Expect(templateVars).Should(HaveKeyWithValue(compSvcVarName2, svcName2))
Expect(templateVars).Should(HaveKeyWithValue(compSvcVarName3, svcName3))
checkEnvVarWithValue(envVars, "service-host", "")
checkEnvVarWithValue(envVars, compSvcVarName1, svcName1)
checkEnvVarWithValue(envVars, compSvcVarName2, svcName2)
checkEnvVarWithValue(envVars, compSvcVarName3, svcName3)
// remove service for comp3
reader.objs = reader.objs[:len(reader.objs)-1]

By("combined - optional partial objects")
vars = []appsv1alpha1.EnvVar{
{
Name: "service-host",
ValueFrom: &appsv1alpha1.VarSource{
ServiceVarRef: &appsv1alpha1.ServiceVarSelector{
ClusterObjectReference: appsv1alpha1.ClusterObjectReference{
CompDef: synthesizedComp.CompDefName,
Name: "service",
Optional: optional(),
MultipleClusterObjectOption: &appsv1alpha1.MultipleClusterObjectOption{
Strategy: appsv1alpha1.MultipleClusterObjectStrategyCombined,
},
Expand All @@ -1285,15 +1338,66 @@ var _ = Describe("vars", func() {
templateVars, envVars, err = ResolveTemplateNEnvVars(testCtx.Ctx, reader, synthesizedComp, nil, vars)
Expect(err).Should(Succeed())
// the combined value will have comp3 in it, but its value will be empty: "comp1:val1,comp2:val2,comp3:"
Expect(templateVars).Should(HaveKeyWithValue("service-host", combinedSvcVarValueWithComp3))
Expect(templateVars).Should(HaveKeyWithValue("service-host", combinedSvcVarValueWithComp3KeyOnly))
Expect(templateVars).ShouldNot(HaveKey(compSvcVarName1))
Expect(templateVars).ShouldNot(HaveKey(compSvcVarName2))
Expect(templateVars).ShouldNot(HaveKey(compSvcVarName3))
// the combined value will have comp3 in it, but its value will be empty: "comp1:val1,comp2:val2,comp3:"
checkEnvVarWithValue(envVars, "service-host", combinedSvcVarValueWithComp3KeyOnly)
checkEnvVarNotExist(envVars, compSvcVarName1)
checkEnvVarNotExist(envVars, compSvcVarName2)
checkEnvVarNotExist(envVars, compSvcVarName3)

By("combined - required partial objects")
vars = []appsv1alpha1.EnvVar{
{
Name: "service-host",
ValueFrom: &appsv1alpha1.VarSource{
ServiceVarRef: &appsv1alpha1.ServiceVarSelector{
ClusterObjectReference: appsv1alpha1.ClusterObjectReference{
CompDef: synthesizedComp.CompDefName,
Name: "service",
Optional: required(), // required
MultipleClusterObjectOption: &appsv1alpha1.MultipleClusterObjectOption{
Strategy: appsv1alpha1.MultipleClusterObjectStrategyCombined,
},
},
ServiceVars: appsv1alpha1.ServiceVars{
Host: &appsv1alpha1.VarRequired,
},
},
},
},
}
_, _, err = ResolveTemplateNEnvVars(testCtx.Ctx, reader, synthesizedComp, nil, vars)
Expect(err).ShouldNot(BeNil())
Expect(err.Error()).Should(ContainSubstring("not found when resolving vars"))
// create service for comp3
reader.objs = append(reader.objs, &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Namespace: testCtx.DefaultNamespace,
Name: svcName3,
},
Spec: corev1.ServiceSpec{
Ports: []corev1.ServicePort{
{
Port: int32(3306),
},
},
},
})
templateVars, envVars, err = ResolveTemplateNEnvVars(testCtx.Ctx, reader, synthesizedComp, nil, vars)
Expect(err).Should(Succeed())
Expect(templateVars).Should(HaveKeyWithValue("service-host", combinedSvcVarValueWithComp3))
Expect(templateVars).ShouldNot(HaveKey(compSvcVarName1))
Expect(templateVars).ShouldNot(HaveKey(compSvcVarName2))
Expect(templateVars).ShouldNot(HaveKey(compSvcVarName3))
checkEnvVarWithValue(envVars, "service-host", combinedSvcVarValueWithComp3)
checkEnvVarNotExist(envVars, compSvcVarName1)
checkEnvVarNotExist(envVars, compSvcVarName2)
checkEnvVarNotExist(envVars, compSvcVarName3)
// remove service for comp3
reader.objs = reader.objs[:len(reader.objs)-1]
})

It("vars reference and escaping", func() {
Expand Down

0 comments on commit 251f11f

Please sign in to comment.