diff --git a/internal/servers/clusters_server.go b/internal/servers/clusters_server.go index 27697d22c..bece99ebf 100644 --- a/internal/servers/clusters_server.go +++ b/internal/servers/clusters_server.go @@ -25,6 +25,7 @@ import ( "google.golang.org/genproto/googleapis/api/httpbody" grpccodes "google.golang.org/grpc/codes" grpcstatus "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/anypb" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -265,6 +266,113 @@ func (s *ClustersServer) Create(ctx context.Context, } cluster.GetSpec().SetNodeSets(actualNodeSets) + // Check that all the specified template parameters are in the template: + templateParameters := template.GetParameters() + clusterParameters := cluster.GetSpec().GetTemplateParameters() + var invalidParameterNames []string + for clusterParameterName := range clusterParameters { + clusterParameterValid := false + for _, templateParameter := range templateParameters { + if templateParameter.GetName() == clusterParameterName { + clusterParameterValid = true + break + } + } + if !clusterParameterValid { + invalidParameterNames = append(invalidParameterNames, clusterParameterName) + } + } + if len(invalidParameterNames) > 0 { + templateParameterNames := make([]string, len(templateParameters)) + for i, templateParameter := range templateParameters { + templateParameterNames[i] = templateParameter.GetName() + } + sort.Strings(templateParameterNames) + for i, templateParameterName := range templateParameterNames { + templateParameterNames[i] = fmt.Sprintf("'%s'", templateParameterName) + } + sort.Strings(invalidParameterNames) + for i, invalidParameterName := range invalidParameterNames { + invalidParameterNames[i] = fmt.Sprintf("'%s'", invalidParameterName) + } + if len(invalidParameterNames) == 1 { + err = grpcstatus.Errorf( + grpccodes.InvalidArgument, + "template parameter %s doesn't exist, valid values for template '%s' are %s", + invalidParameterNames[0], + templateId, + english.WordSeries(templateParameterNames, "and"), + ) + } else { + err = grpcstatus.Errorf( + grpccodes.InvalidArgument, + "template parameters %s don't exist, valid values for template '%s' are %s", + english.WordSeries(invalidParameterNames, "and"), + templateId, + english.WordSeries(templateParameterNames, "and"), + ) + } + return + } + + // Check that all the mandatory parameters have a value: + for _, templateParameter := range templateParameters { + if !templateParameter.GetRequired() { + continue + } + templateParameterName := templateParameter.GetName() + clusterParameter := clusterParameters[templateParameterName] + if clusterParameter == nil { + err = grpcstatus.Errorf( + grpccodes.InvalidArgument, + "parameter '%s' of template '%s' is mandatory", + templateParameterName, templateId, + ) + return + } + } + + // Check that the parameter values are compatible with the template: + for clusterParameterName, clusterParameter := range clusterParameters { + for _, templateParameter := range templateParameters { + templateParameterName := templateParameter.GetName() + if clusterParameterName != templateParameterName { + continue + } + clusterParameterType := clusterParameter.GetTypeUrl() + templateParameterType := templateParameter.GetType() + if clusterParameterType != templateParameterType { + err = grpcstatus.Errorf( + grpccodes.InvalidArgument, + "type of parameter '%s' of template '%s' should be '%s', "+ + "but it is '%s'", + clusterParameterName, + templateId, + templateParameterType, + clusterParameterType, + ) + return + } + } + } + + // Set default values for template parameters: + actualClusterParameters := make(map[string]*anypb.Any) + for _, templateParameter := range templateParameters { + templateParameterName := templateParameter.GetName() + clusterParameter := clusterParameters[templateParameterName] + actualClusterParameter := &anypb.Any{ + TypeUrl: templateParameter.GetType(), + } + if clusterParameter != nil { + actualClusterParameter.Value = clusterParameter.Value + } else { + actualClusterParameter.Value = templateParameter.GetDefault().GetValue() + } + actualClusterParameters[templateParameterName] = actualClusterParameter + } + cluster.GetSpec().SetTemplateParameters(actualClusterParameters) + err = s.generic.Create(ctx, request, &response) return } diff --git a/internal/servers/clusters_server_test.go b/internal/servers/clusters_server_test.go index a47329728..6f2f8b334 100644 --- a/internal/servers/clusters_server_test.go +++ b/internal/servers/clusters_server_test.go @@ -23,6 +23,8 @@ import ( grpccodes "google.golang.org/grpc/codes" grpcstatus "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/anypb" + "google.golang.org/protobuf/types/known/wrapperspb" ffv1 "github.com/innabox/fulfillment-service/internal/api/fulfillment/v1" privatev1 "github.com/innabox/fulfillment-service/internal/api/private/v1" @@ -125,6 +127,12 @@ var _ = Describe("Clusters server", func() { Describe("Behaviour", func() { var server *ClustersServer + makeAny := func(m proto.Message) *anypb.Any { + a, err := anypb.New(m) + Expect(err).ToNot(HaveOccurred()) + return a + } + BeforeEach(func() { var err error @@ -173,6 +181,31 @@ var _ = Describe("Clusters server", func() { Expect(err).ToNot(HaveOccurred()) err = templatesDao.Delete(ctx, "my_deleted_template") Expect(err).ToNot(HaveOccurred()) + + // Create a template with parameters: + _, err = templatesDao.Create(ctx, privatev1.ClusterTemplate_builder{ + Id: "my_with_parameters", + Title: "My with parameters", + Description: "My with parameters.", + Parameters: []*privatev1.ClusterTemplateParameterDefinition{ + privatev1.ClusterTemplateParameterDefinition_builder{ + Name: "my_required_bool", + Title: "My required bool", + Description: "My required bool.", + Required: true, + Type: "type.googleapis.com/google.protobuf.BoolValue", + }.Build(), + privatev1.ClusterTemplateParameterDefinition_builder{ + Name: "my_optional_string", + Title: "My optional string", + Description: "My optional string.", + Required: false, + Type: "type.googleapis.com/google.protobuf.StringValue", + Default: makeAny(wrapperspb.String("my value")), + }.Build(), + }, + }.Build()) + Expect(err).ToNot(HaveOccurred()) }) It("Creates object", func() { @@ -409,6 +442,145 @@ var _ = Describe("Clusters server", func() { )) }) + It("Doesn't create object if there are missing required template parameters", func() { + response, err := server.Create(ctx, ffv1.ClustersCreateRequest_builder{ + Object: ffv1.Cluster_builder{ + Spec: ffv1.ClusterSpec_builder{ + Template: "my_with_parameters", + }.Build(), + }.Build(), + }.Build()) + Expect(err).To(HaveOccurred()) + Expect(response).To(BeNil()) + status, ok := grpcstatus.FromError(err) + Expect(ok).To(BeTrue()) + Expect(status.Code()).To(Equal(grpccodes.InvalidArgument)) + Expect(status.Message()).To(Equal( + "parameter 'my_required_bool' of template 'my_with_parameters' is mandatory", + )) + }) + + It("Doesn't create object if one parameter doesn't exist in the template", func() { + response, err := server.Create(ctx, ffv1.ClustersCreateRequest_builder{ + Object: ffv1.Cluster_builder{ + Spec: ffv1.ClusterSpec_builder{ + Template: "my_with_parameters", + TemplateParameters: map[string]*anypb.Any{ + "junk": makeAny(wrapperspb.Int32(123)), + }, + }.Build(), + }.Build(), + }.Build()) + Expect(err).To(HaveOccurred()) + Expect(response).To(BeNil()) + status, ok := grpcstatus.FromError(err) + Expect(ok).To(BeTrue()) + Expect(status.Code()).To(Equal(grpccodes.InvalidArgument)) + Expect(status.Message()).To(Equal( + "template parameter 'junk' doesn't exist, valid values for template " + + "'my_with_parameters' are 'my_optional_string' and 'my_required_bool'", + )) + }) + + It("Doesn't create object if two parameters don't exist in the template", func() { + response, err := server.Create(ctx, ffv1.ClustersCreateRequest_builder{ + Object: ffv1.Cluster_builder{ + Spec: ffv1.ClusterSpec_builder{ + Template: "my_with_parameters", + TemplateParameters: map[string]*anypb.Any{ + "junk1": makeAny(wrapperspb.Int32(123)), + "junk2": makeAny(wrapperspb.Int32(123)), + }, + }.Build(), + }.Build(), + }.Build()) + Expect(err).To(HaveOccurred()) + Expect(response).To(BeNil()) + status, ok := grpcstatus.FromError(err) + Expect(ok).To(BeTrue()) + Expect(status.Code()).To(Equal(grpccodes.InvalidArgument)) + Expect(status.Message()).To(Equal( + "template parameters 'junk1' and 'junk2' don't exist, valid values for template " + + "'my_with_parameters' are 'my_optional_string' and 'my_required_bool'", + )) + }) + + It("Doesn't create object if parameter type doesn't match the template", func() { + response, err := server.Create(ctx, ffv1.ClustersCreateRequest_builder{ + Object: ffv1.Cluster_builder{ + Spec: ffv1.ClusterSpec_builder{ + Template: "my_with_parameters", + TemplateParameters: map[string]*anypb.Any{ + "my_required_bool": makeAny(wrapperspb.Int32(123)), + }, + }.Build(), + }.Build(), + }.Build()) + Expect(err).To(HaveOccurred()) + Expect(response).To(BeNil()) + status, ok := grpcstatus.FromError(err) + Expect(ok).To(BeTrue()) + Expect(status.Code()).To(Equal(grpccodes.InvalidArgument)) + Expect(status.Message()).To(Equal( + "type of parameter 'my_required_bool' of template 'my_with_parameters' should be " + + "'type.googleapis.com/google.protobuf.BoolValue', but it is " + + "'type.googleapis.com/google.protobuf.Int32Value'", + )) + }) + + It("Takes default values of parameters from the template", func() { + response, err := server.Create(ctx, ffv1.ClustersCreateRequest_builder{ + Object: ffv1.Cluster_builder{ + Spec: ffv1.ClusterSpec_builder{ + Template: "my_with_parameters", + TemplateParameters: map[string]*anypb.Any{ + "my_required_bool": makeAny(wrapperspb.Bool(true)), + }, + }.Build(), + }.Build(), + }.Build()) + Expect(err).ToNot(HaveOccurred()) + object := response.GetObject() + templateParameters := object.GetSpec().GetTemplateParameters() + + parameterValue := templateParameters["my_required_bool"] + Expect(parameterValue).ToNot(BeNil()) + boolValue := &wrapperspb.BoolValue{} + err = anypb.UnmarshalTo(parameterValue, boolValue, proto.UnmarshalOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(boolValue.GetValue()).To(BeTrue()) + + parameterValue = templateParameters["my_optional_string"] + Expect(parameterValue).ToNot(BeNil()) + stringValue := &wrapperspb.StringValue{} + err = anypb.UnmarshalTo(parameterValue, stringValue, proto.UnmarshalOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(stringValue.GetValue()).To(Equal("my value")) + }) + + It("Allows overriding of default values of template parameters", func() { + response, err := server.Create(ctx, ffv1.ClustersCreateRequest_builder{ + Object: ffv1.Cluster_builder{ + Spec: ffv1.ClusterSpec_builder{ + Template: "my_with_parameters", + TemplateParameters: map[string]*anypb.Any{ + "my_required_bool": makeAny(wrapperspb.Bool(false)), + "my_optional_string": makeAny(wrapperspb.String("your value")), + }, + }.Build(), + }.Build(), + }.Build()) + Expect(err).ToNot(HaveOccurred()) + object := response.GetObject() + templateParameters := object.GetSpec().GetTemplateParameters() + parameterValue := templateParameters["my_optional_string"] + Expect(parameterValue).ToNot(BeNil()) + stringValue := &wrapperspb.StringValue{} + err = anypb.UnmarshalTo(parameterValue, stringValue, proto.UnmarshalOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(stringValue.GetValue()).To(Equal("your value")) + }) + It("List objects", func() { // Create a few objects: const count = 10