From 0c56848ea06b78dbabd253c651b50e523976d59c Mon Sep 17 00:00:00 2001 From: Lency Qian <36918585+LeiQL@users.noreply.github.com> Date: Mon, 22 Jan 2024 10:16:41 +0800 Subject: [PATCH 1/2] guard request ID null in state providers --- .../apis/v1alpha1/providers/states/k8s/k8s.go | 16 +++++- .../v1alpha1/providers/states/k8s/k8s_test.go | 2 +- .../providers/states/httpstate/httpstate.go | 18 +++++- .../states/httpstate/httpstate_test.go | 56 +++++++++++++++++++ 4 files changed, 87 insertions(+), 5 deletions(-) diff --git a/api/pkg/apis/v1alpha1/providers/states/k8s/k8s.go b/api/pkg/apis/v1alpha1/providers/states/k8s/k8s.go index 1a4d8cbe3..4b68bdf3c 100644 --- a/api/pkg/apis/v1alpha1/providers/states/k8s/k8s.go +++ b/api/pkg/apis/v1alpha1/providers/states/k8s/k8s.go @@ -189,8 +189,12 @@ func (s *K8sStateProvider) Upsert(ctx context.Context, entry states.UpsertReques Resource: resource, } - j, _ := json.Marshal(entry.Value.Body) + if entry.Value.ID == "" { + err := v1alpha2.NewCOAError(nil, "found invalid request ID", v1alpha2.InternalError) + return "", err + } + j, _ := json.Marshal(entry.Value.Body) item, err := s.DynamicClient.Resource(resourceId).Namespace(scope).Get(ctx, entry.Value.ID, metav1.GetOptions{}) if err != nil { // TODO: check if not-found error @@ -355,6 +359,11 @@ func (s *K8sStateProvider) Delete(ctx context.Context, request states.DeleteRequ scope = "default" } + if request.ID == "" { + err := v1alpha2.NewCOAError(nil, "found invalid request ID", v1alpha2.InternalError) + return err + } + err = s.DynamicClient.Resource(resourceId).Namespace(scope).Delete(ctx, request.ID, metav1.DeleteOptions{}) if err != nil { sLog.Errorf(" P (K8s State): failed to delete objects: %v", err) @@ -387,6 +396,11 @@ func (s *K8sStateProvider) Get(ctx context.Context, request states.GetRequest) ( Resource: resource, } + if request.ID == "" { + err := v1alpha2.NewCOAError(nil, "found invalid request ID", v1alpha2.InternalError) + return states.StateEntry{}, err + } + item, err := s.DynamicClient.Resource(resourceId).Namespace(scope).Get(ctx, request.ID, metav1.GetOptions{}) if err != nil { coaError := v1alpha2.NewCOAError(err, "failed to get object", v1alpha2.InternalError) diff --git a/api/pkg/apis/v1alpha1/providers/states/k8s/k8s_test.go b/api/pkg/apis/v1alpha1/providers/states/k8s/k8s_test.go index 11f78db01..6bd945ada 100644 --- a/api/pkg/apis/v1alpha1/providers/states/k8s/k8s_test.go +++ b/api/pkg/apis/v1alpha1/providers/states/k8s/k8s_test.go @@ -72,7 +72,7 @@ func TestInitWithBadData(t *testing.T) { assert.NotNil(t, err) } -func TestUpSert(t *testing.T) { +func TestUpsert(t *testing.T) { testK8s := os.Getenv("TEST_K8S_STATE") if testK8s == "" { t.Skip("Skipping because TEST_K8S_STATE enviornment variable is not set") diff --git a/coa/pkg/apis/v1alpha2/providers/states/httpstate/httpstate.go b/coa/pkg/apis/v1alpha2/providers/states/httpstate/httpstate.go index e31e9dea2..a2570afb4 100644 --- a/coa/pkg/apis/v1alpha2/providers/states/httpstate/httpstate.go +++ b/coa/pkg/apis/v1alpha2/providers/states/httpstate/httpstate.go @@ -115,16 +115,21 @@ func (s *HttpStateProvider) Init(config providers.IProviderConfig) error { } s.Config = stateConfig if s.Config.Url == "" { - return v1alpha2.NewCOAError(nil, "Http sate provider url is not set", v1alpha2.BadConfig) + return v1alpha2.NewCOAError(nil, "Http state provider url is not set", v1alpha2.BadConfig) } s.Data = make(map[string]interface{}, 0) return nil } func (s *HttpStateProvider) Upsert(ctx context.Context, entry states.UpsertRequest) (string, error) { + client := &http.Client{} rUrl := s.Config.Url var err error + if entry.Value.ID == "" { + err = v1alpha2.NewCOAError(nil, "found invalid entry ID", v1alpha2.InternalError) + return "", err + } if s.Config.PostNameInPath { rUrl, err = url.JoinPath(s.Config.Url, entry.Value.ID) } @@ -157,11 +162,15 @@ func (s *HttpStateProvider) Upsert(ctx context.Context, entry states.UpsertReque } func (s *HttpStateProvider) List(ctx context.Context, request states.ListRequest) ([]states.StateEntry, string, error) { - return nil, "", v1alpha2.NewCOAError(nil, "Http sate store list is not implemented", v1alpha2.NotImplemented) + return nil, "", v1alpha2.NewCOAError(nil, "Http state store list is not implemented", v1alpha2.NotImplemented) } func (s *HttpStateProvider) Delete(ctx context.Context, request states.DeleteRequest) error { client := &http.Client{} + if request.ID == "" { + err := v1alpha2.NewCOAError(nil, "found invalid request ID", v1alpha2.InternalError) + return err + } rUrl, err := url.JoinPath(s.Config.Url, request.ID) if err != nil { return err @@ -176,13 +185,16 @@ func (s *HttpStateProvider) Delete(ctx context.Context, request states.DeleteReq } if resp.StatusCode >= 300 { return v1alpha2.NewCOAError(nil, fmt.Sprintf("failed to delete from HTTP state store: [%d]", resp.StatusCode), v1alpha2.InternalError) - } return nil } func (s *HttpStateProvider) Get(ctx context.Context, request states.GetRequest) (states.StateEntry, error) { client := &http.Client{} + if request.ID == "" { + err := v1alpha2.NewCOAError(nil, "found invalid request ID", v1alpha2.InternalError) + return states.StateEntry{}, err + } rUrl, err := url.JoinPath(s.Config.Url, request.ID) if err != nil { return states.StateEntry{}, err diff --git a/coa/pkg/apis/v1alpha2/providers/states/httpstate/httpstate_test.go b/coa/pkg/apis/v1alpha2/providers/states/httpstate/httpstate_test.go index d52dfbec7..5c06fdec6 100644 --- a/coa/pkg/apis/v1alpha2/providers/states/httpstate/httpstate_test.go +++ b/coa/pkg/apis/v1alpha2/providers/states/httpstate/httpstate_test.go @@ -9,6 +9,7 @@ package httpstate import ( "context" "encoding/json" + "io/ioutil" "net/http" "net/http/httptest" "os" @@ -237,12 +238,48 @@ func TestGet(t *testing.T) { func TestUpsertGetDelete(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method == "GET" { + relativePath := r.URL.Path + if relativePath == "" || relativePath == "/" { + http.Error(w, "invalid path", http.StatusBadRequest) + return + } response := map[string]interface{}{ "key": "abc", } jsonResponse, _ := json.Marshal(response) w.Header().Set("Content-Type", "application/json") w.Write(jsonResponse) + } else if r.Method == "POST" { + var data []map[string]interface{} + body, _ := ioutil.ReadAll(r.Body) + err := json.Unmarshal([]byte(body), &data) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + if len(data) == 0 { + http.Error(w, "invalid request body", http.StatusBadRequest) + return + } + firstItemKey := data[0]["key"].(string) + if firstItemKey == "" { + http.Error(w, "invalid request body", http.StatusBadRequest) + return + } + response := map[string]interface{}{ + "key": data[0]["key"], + "value": data[0]["value"], + } + jsonResponse, _ := json.Marshal(response) + w.Header().Set("Content-Type", "application/json") + w.Write(jsonResponse) + } else if r.Method == "DELETE" { + relativePath := r.URL.Path + if relativePath == "" || relativePath == "/" { + http.Error(w, "invalid path", http.StatusBadRequest) + return + } + w.Write([]byte("OK")) } else { w.Write([]byte("OK")) } @@ -258,6 +295,7 @@ func TestUpsertGetDelete(t *testing.T) { PostAsArray: true, NotFoundAs204: true, }) + assert.Nil(t, err) _, err = provider.Upsert(context.Background(), states.UpsertRequest{ Value: states.StateEntry{ @@ -277,6 +315,24 @@ func TestUpsertGetDelete(t *testing.T) { ID: "123", }) assert.Nil(t, err) + + _, err = provider.Upsert(context.Background(), states.UpsertRequest{ + Value: states.StateEntry{ + ID: "", + Body: TestPayload{}, + }, + }) + assert.NotNil(t, err) + + _, err = provider.Get(context.Background(), states.GetRequest{ + ID: "", + }) + assert.NotNil(t, err) + + err = provider.Delete(context.Background(), states.DeleteRequest{ + ID: "", + }) + assert.NotNil(t, err) } func TestClone(t *testing.T) { From f9cd855561a8f86536370ed9a191abec8e95e863 Mon Sep 17 00:00:00 2001 From: Lency Qian Date: Tue, 30 Jan 2024 11:13:37 +0800 Subject: [PATCH 2/2] handle badrequest --- api/pkg/apis/v1alpha1/providers/states/k8s/k8s.go | 6 +++--- .../apis/v1alpha2/providers/states/httpstate/httpstate.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api/pkg/apis/v1alpha1/providers/states/k8s/k8s.go b/api/pkg/apis/v1alpha1/providers/states/k8s/k8s.go index 4b68bdf3c..860238fcc 100644 --- a/api/pkg/apis/v1alpha1/providers/states/k8s/k8s.go +++ b/api/pkg/apis/v1alpha1/providers/states/k8s/k8s.go @@ -190,7 +190,7 @@ func (s *K8sStateProvider) Upsert(ctx context.Context, entry states.UpsertReques } if entry.Value.ID == "" { - err := v1alpha2.NewCOAError(nil, "found invalid request ID", v1alpha2.InternalError) + err := v1alpha2.NewCOAError(nil, "found invalid request ID", v1alpha2.BadRequest) return "", err } @@ -360,7 +360,7 @@ func (s *K8sStateProvider) Delete(ctx context.Context, request states.DeleteRequ } if request.ID == "" { - err := v1alpha2.NewCOAError(nil, "found invalid request ID", v1alpha2.InternalError) + err := v1alpha2.NewCOAError(nil, "found invalid request ID", v1alpha2.BadRequest) return err } @@ -397,7 +397,7 @@ func (s *K8sStateProvider) Get(ctx context.Context, request states.GetRequest) ( } if request.ID == "" { - err := v1alpha2.NewCOAError(nil, "found invalid request ID", v1alpha2.InternalError) + err := v1alpha2.NewCOAError(nil, "found invalid request ID", v1alpha2.BadRequest) return states.StateEntry{}, err } diff --git a/coa/pkg/apis/v1alpha2/providers/states/httpstate/httpstate.go b/coa/pkg/apis/v1alpha2/providers/states/httpstate/httpstate.go index a2570afb4..191ebc809 100644 --- a/coa/pkg/apis/v1alpha2/providers/states/httpstate/httpstate.go +++ b/coa/pkg/apis/v1alpha2/providers/states/httpstate/httpstate.go @@ -127,7 +127,7 @@ func (s *HttpStateProvider) Upsert(ctx context.Context, entry states.UpsertReque rUrl := s.Config.Url var err error if entry.Value.ID == "" { - err = v1alpha2.NewCOAError(nil, "found invalid entry ID", v1alpha2.InternalError) + err = v1alpha2.NewCOAError(nil, "found invalid entry ID", v1alpha2.BadRequest) return "", err } if s.Config.PostNameInPath { @@ -168,7 +168,7 @@ func (s *HttpStateProvider) List(ctx context.Context, request states.ListRequest func (s *HttpStateProvider) Delete(ctx context.Context, request states.DeleteRequest) error { client := &http.Client{} if request.ID == "" { - err := v1alpha2.NewCOAError(nil, "found invalid request ID", v1alpha2.InternalError) + err := v1alpha2.NewCOAError(nil, "found invalid request ID", v1alpha2.BadRequest) return err } rUrl, err := url.JoinPath(s.Config.Url, request.ID) @@ -192,7 +192,7 @@ func (s *HttpStateProvider) Delete(ctx context.Context, request states.DeleteReq func (s *HttpStateProvider) Get(ctx context.Context, request states.GetRequest) (states.StateEntry, error) { client := &http.Client{} if request.ID == "" { - err := v1alpha2.NewCOAError(nil, "found invalid request ID", v1alpha2.InternalError) + err := v1alpha2.NewCOAError(nil, "found invalid request ID", v1alpha2.BadRequest) return states.StateEntry{}, err } rUrl, err := url.JoinPath(s.Config.Url, request.ID)