diff --git a/pkg/api/resource_types.go b/pkg/api/resource_types.go index 4b1d8e83..e2c06387 100755 --- a/pkg/api/resource_types.go +++ b/pkg/api/resource_types.go @@ -165,26 +165,11 @@ func EncodeManifest(manifest, deleteOption, updateStrategy map[string]interface{ return nil, nil } - delOption := &workv1.DeleteOption{ - PropagationPolicy: workv1.DeletePropagationPolicyTypeForeground, - } - if len(deleteOption) != 0 { - delOption = &workv1.DeleteOption{} - deleteOptionBytes, err := json.Marshal(deleteOption) - if err != nil { - return nil, fmt.Errorf("failed to marshal deleteOption to json: %v", err) - } - err = json.Unmarshal(deleteOptionBytes, delOption) - if err != nil { - return nil, fmt.Errorf("failed to unmarshal json to deleteOption: %v", err) - } - } - + // default update strategy is ServerSideApply upStrategy := &workv1.UpdateStrategy{ Type: workv1.UpdateStrategyTypeServerSideApply, } if len(updateStrategy) != 0 { - upStrategy = &workv1.UpdateStrategy{} updateStrategyBytes, err := json.Marshal(updateStrategy) if err != nil { return nil, fmt.Errorf("failed to marshal updateStrategy to json: %v", err) @@ -193,7 +178,29 @@ func EncodeManifest(manifest, deleteOption, updateStrategy map[string]interface{ if err != nil { return nil, fmt.Errorf("failed to unmarshal json to updateStrategy: %v", err) } - fmt.Println("upStrategy", upStrategy) + } + + // default delete option is Foreground + delOption := &workv1.DeleteOption{ + PropagationPolicy: workv1.DeletePropagationPolicyTypeForeground, + } + + // set delete option to Orphan if update strategy is ReadOnly + if upStrategy.Type == workv1.UpdateStrategyTypeReadOnly { + delOption = &workv1.DeleteOption{ + PropagationPolicy: workv1.DeletePropagationPolicyTypeOrphan, + } + } else { + if len(deleteOption) != 0 { + deleteOptionBytes, err := json.Marshal(deleteOption) + if err != nil { + return nil, fmt.Errorf("failed to marshal deleteOption to json: %v", err) + } + err = json.Unmarshal(deleteOptionBytes, delOption) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal json to deleteOption: %v", err) + } + } } // create a cloud event with the manifest as the data diff --git a/pkg/handlers/resource.go b/pkg/handlers/resource.go index 501ddc60..555462c9 100755 --- a/pkg/handlers/resource.go +++ b/pkg/handlers/resource.go @@ -35,6 +35,7 @@ func (h resourceHandler) Create(w http.ResponseWriter, r *http.Request) { validateEmpty(&rs, "Id", "id"), validateNotEmpty(&rs, "ConsumerName", "consumer_name"), validateNotEmpty(&rs, "Manifest", "manifest"), + validateDeleteOptionAndUpdateStrategy(&rs), }, func() (interface{}, *errors.ServiceError) { ctx := r.Context() diff --git a/pkg/handlers/validation.go b/pkg/handlers/validation.go index 48f6c6d6..4367c525 100755 --- a/pkg/handlers/validation.go +++ b/pkg/handlers/validation.go @@ -3,6 +3,7 @@ package handlers import ( "reflect" + "github.com/openshift-online/maestro/pkg/api/openapi" "github.com/openshift-online/maestro/pkg/errors" ) @@ -37,3 +38,24 @@ func validateEmpty(i interface{}, fieldName string, field string) validate { return nil } } + +// validateDeleteOptionAndUpdateStrategy validates the delete option and update strategy +// for a resource, to ensure that update strategy ReadOnly is only allowed with delete option Orphan. +func validateDeleteOptionAndUpdateStrategy(rs *openapi.Resource) validate { + return func() *errors.ServiceError { + if rs.DeleteOption != nil && rs.UpdateStrategy != nil { + deleteType, ok := rs.DeleteOption["propagationPolicy"].(string) + if !ok { + return errors.Validation("invalid delete option") + } + updateStrategy, ok := rs.UpdateStrategy["type"].(string) + if !ok { + return errors.Validation("invalid update strategy") + } + if deleteType != "Orphan" && updateStrategy == "ReadOnly" { + return errors.Validation("update strategy ReadOnly is only allowed with delete option Orphan") + } + } + return nil + } +} diff --git a/test/e2e/pkg/resources_test.go b/test/e2e/pkg/resources_test.go index 2ca7aef8..fb190b7c 100644 --- a/test/e2e/pkg/resources_test.go +++ b/test/e2e/pkg/resources_test.go @@ -227,9 +227,16 @@ var _ = Describe("Resources", Ordered, Label("e2e-tests-resources"), func() { }) It("post the resource to the maestro api with readonly updateStrategy", func() { - res := helper.NewReadOnlyAPIResource(consumer.Name, deployName) var resp *http.Response var err error + // post the resource with readonly updateStrategy and foreground delete option should fail + invalidRes := helper.NewReadOnlyAPIResource(consumer.Name, deployName) + invalidRes.DeleteOption = map[string]interface{}{"propagationPolicy": "Foreground"} + resource, resp, err = apiClient.DefaultApi.ApiMaestroV1ResourcesPost(ctx).Resource(invalidRes).Execute() + Expect(err).Should(HaveOccurred()) + Expect(resp.StatusCode).To(Equal(http.StatusBadRequest)) + + res := helper.NewReadOnlyAPIResource(consumer.Name, deployName) resource, resp, err = apiClient.DefaultApi.ApiMaestroV1ResourcesPost(ctx).Resource(res).Execute() Expect(err).ShouldNot(HaveOccurred()) Expect(resp.StatusCode).To(Equal(http.StatusCreated)) @@ -243,6 +250,15 @@ var _ = Describe("Resources", Ordered, Label("e2e-tests-resources"), func() { return err } + // ensure the delete option is set to Orphan + deleteType, ok := res.DeleteOption["propagationPolicy"] + if !ok { + return fmt.Errorf("delete option is not set") + } + if deleteType != "Orphan" { + return fmt.Errorf("delete option is not Orphan") + } + statusJSON, err := json.Marshal(res.Status) if err != nil { return err diff --git a/test/factories.go b/test/factories.go index b1ba8678..85a6f97e 100755 --- a/test/factories.go +++ b/test/factories.go @@ -73,9 +73,6 @@ var testReadOnlyManifestJSON = ` "metadata": { "name": "%s", "namespace": "%s" - }, - "update_strategy": { - "type": "ReadOnly" } } ` @@ -125,6 +122,9 @@ func (helper *Helper) NewReadOnlyAPIResource(consumerName, deployName string) op return openapi.Resource{ Manifest: testManifest, ConsumerName: &consumerName, + UpdateStrategy: map[string]interface{}{ + "type": "ReadOnly", + }, } }