Skip to content

Commit

Permalink
default orphan delete option for read only update strategy.
Browse files Browse the repository at this point in the history
Signed-off-by: morvencao <[email protected]>
  • Loading branch information
morvencao committed Sep 6, 2024
1 parent e9b98cc commit 8491d18
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 21 deletions.
41 changes: 24 additions & 17 deletions pkg/api/resource_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 default delete option to Orphan if update strategy is ReadOnly
if upStrategy.Type == workv1.UpdateStrategyTypeReadOnly {
delOption = &workv1.DeleteOption{
PropagationPolicy: workv1.DeletePropagationPolicyTypeOrphan,
}
}

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
Expand Down
1 change: 1 addition & 0 deletions pkg/handlers/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
22 changes: 22 additions & 0 deletions pkg/handlers/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package handlers
import (
"reflect"

"github.com/openshift-online/maestro/pkg/api/openapi"
"github.com/openshift-online/maestro/pkg/errors"
)

Expand Down Expand Up @@ -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
}
}
18 changes: 17 additions & 1 deletion test/e2e/pkg/resources_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions test/factories.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,6 @@ var testReadOnlyManifestJSON = `
"metadata": {
"name": "%s",
"namespace": "%s"
},
"update_strategy": {
"type": "ReadOnly"
}
}
`
Expand Down Expand Up @@ -114,6 +111,9 @@ func (helper *Helper) NewReadOnlyAPIResource(consumerName, deployName string) op
return openapi.Resource{
Manifest: testManifest,
ConsumerName: &consumerName,
UpdateStrategy: map[string]interface{}{
"type": "ReadOnly",
},
}
}

Expand Down

0 comments on commit 8491d18

Please sign in to comment.