Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API OperatorFlagsMergeStrategy #2753

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions pkg/api/admin/operatorflags.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package admin

// Copyright (c) Microsoft Corporation.
// Licensed under the Apache License 2.0.

import (
"encoding/json"
"net/http"

"github.com/Azure/ARO-RP/pkg/api"
"github.com/Azure/ARO-RP/pkg/operator"
)

type operatorFlagsMergeStrategyStruct struct {

Check failure on line 14 in pkg/api/admin/operatorflags.go

View workflow job for this annotation

GitHub Actions / golangci-lint

`operatorFlagsMergeStrategyStruct` should be annotated with the `json` tag as it is passed to `json.Unmarshal` at pkg/api/admin/operatorflags.go:38:9 (musttag)
SrinivasAtmakuri marked this conversation as resolved.
Show resolved Hide resolved
OperatorFlagsMergeStrategy string
Cluster *api.OpenShiftCluster
}

const (
operatorFlagsMergeStrategyDefault string = "merge"
operatorFlagsMergeStrategyMerge string = "merge"
operatorFlagsMergeStrategyReset string = "reset"
)

// When a cluster is edited via the PATCH Cluster Geneva Action (aka an Admin Update)
// the flags given are treated according to the provided Update Strategy,
// provided in operatorFlagsMergeStrategy

// merge (default): The provided cluster flags are laid on top of the cluster’s existing flags.
// reset: The provided cluster flags are laid on top of the default cluster flags,
// essentially ‘resetting’ the flags if no new flags are provided.
func OperatorFlagsMergeStrategy(oc *api.OpenShiftCluster, body []byte) error {
payload := operatorFlagsMergeStrategyStruct{
OperatorFlagsMergeStrategy: operatorFlagsMergeStrategyDefault,
Cluster: &api.OpenShiftCluster{},
}

err := json.Unmarshal(body, &payload)
if err != nil {
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidRequestContent, "", "The request content was invalid and could not be deserialized: %q.", err)
}

// return error if OperatorFlagsMergeStrategy is not merge or reset, default is merge
if payload.OperatorFlagsMergeStrategy != operatorFlagsMergeStrategyMerge &&
payload.OperatorFlagsMergeStrategy != operatorFlagsMergeStrategyReset {
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "", "invalid operatorFlagsMergeStrategy '%s', can only be 'merge' or 'reset'", payload.OperatorFlagsMergeStrategy)
}
// return if payload is empty
if payload.Cluster == nil {
return nil
}
properties := &payload.Cluster.Properties
if properties == nil || properties.OperatorFlags == nil {
return nil
}
// return nil, if OperatorFlagsMergeStrategy is merge and payload has not operatorFlags
// return operatorFlags of payload, if OperatorFlagsMergeStrategy is merge and payload has operatorFlags
// return defaultOperatorFlags, if OperatorFlagsMergeStrategy is reset and payload has not operatorFlags
// return defaultOperatorFlags + operatorFlags of payload, if OperatorFlagsMergeStrategy is reset and payload has operatorFlags
SrinivasAtmakuri marked this conversation as resolved.
Show resolved Hide resolved
petrkotas marked this conversation as resolved.
Show resolved Hide resolved
if payload.OperatorFlagsMergeStrategy == operatorFlagsMergeStrategyReset {
SrinivasAtmakuri marked this conversation as resolved.
Show resolved Hide resolved
oc.Properties.OperatorFlags = operator.DefaultOperatorFlags()
for operatorflag, value := range payload.Cluster.Properties.OperatorFlags {
oc.Properties.OperatorFlags[operatorflag] = value
}
}

return nil
}
60 changes: 60 additions & 0 deletions pkg/api/admin/operatorflags_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package admin

import (
"testing"

"github.com/Azure/ARO-RP/pkg/api"
utilerror "github.com/Azure/ARO-RP/test/util/error"
)

// Copyright (c) Microsoft Corporation.
// Licensed under the Apache License 2.0.

func TestOperatorFlagsMergeStrategy(t *testing.T) {
tests := []struct {
name string
oc *api.OpenShiftCluster
wantOc *api.OpenShiftCluster
body []byte
wantErr string
}{
{
name: "invalid_json",
oc: nil,
body: []byte(`{{}`),
wantErr: `400: InvalidRequestContent: : The request content was invalid and could not be deserialized: "invalid character '{' looking for beginning of object key string".`,
},
{
name: "OperatorFlagsMergeStrategy_is_not_merge_or_reset",
oc: nil,
body: []byte(`{"operatorFlagsMergeStrategy": "xyz"}`),
wantErr: `400: InvalidParameter: : invalid operatorFlagsMergeStrategy 'xyz', can only be 'merge' or 'reset'`,
},
{
name: "OperatorFlagsMergeStrategy_payload_is_empty",
oc: &api.OpenShiftCluster{
Properties: api.OpenShiftClusterProperties{
OperatorFlags: api.OperatorFlags{"aro.feature1.enabled": "false"},
},
},
body: []byte(`{"operatorflagsmergestrategy":"merge"}`),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SrinivasAtmakuri would you please also add the reset strategy, to make it explicit and show that it works please?

wantErr: "",
},
{
name: "OperatorFlagsMergeStrategy_reset",
oc: &api.OpenShiftCluster{
Properties: api.OpenShiftClusterProperties{
OperatorFlags: api.OperatorFlags{"aro.feature1.enabled": "false"},
},
},
body: []byte(`{"operatorflagsmergestrategy":"reset"}`),
wantErr: "",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := OperatorFlagsMergeStrategy(tt.oc, tt.body)
utilerror.AssertErrorMessage(t, err, tt.wantErr)
})
}
}
4 changes: 4 additions & 0 deletions pkg/frontend/openshiftcluster_putorpatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,10 @@ func (f *frontend) _putOrPatchOpenShiftCluster(ctx context.Context, log *logrus.
// provide single field json to be updated in the database.
// Patch should be used for updating individual fields of the document.
case http.MethodPatch:
err = admin.OperatorFlagsMergeStrategy(doc.OpenShiftCluster, body)
if err != nil {
return nil, err
}
ext = converter.ToExternal(doc.OpenShiftCluster)
converter.ExternalNoReadOnly(ext)
}
Expand Down
Loading