From 1a07a33ac29d52e7214128389def9370fcfd04d2 Mon Sep 17 00:00:00 2001 From: Salah Aldeen Al Saleh Date: Mon, 9 Oct 2023 22:48:57 +0000 Subject: [PATCH 1/4] Add an endpoint to the SDK where metrics can be retrieved adhoc --- pact/custom_metrics_test.go | 4 +- pkg/apiserver/server.go | 9 ++- pkg/handlers/app.go | 72 ++++++++++++++++++ .../{custom_metrics_test.go => app_test.go} | 16 ++-- pkg/handlers/custom_metrics.go | 74 ------------------- pkg/handlers/middleware.go | 20 +++++ pkg/handlers/types/types.go | 6 ++ pkg/heartbeat/util.go | 30 +++++--- pkg/metrics/metrics.go | 2 +- 9 files changed, 136 insertions(+), 97 deletions(-) rename pkg/handlers/{custom_metrics_test.go => app_test.go} (73%) delete mode 100644 pkg/handlers/custom_metrics.go create mode 100644 pkg/handlers/types/types.go diff --git a/pact/custom_metrics_test.go b/pact/custom_metrics_test.go index 62223774..ea297cc6 100644 --- a/pact/custom_metrics_test.go +++ b/pact/custom_metrics_test.go @@ -16,7 +16,7 @@ import ( "github.com/replicatedhq/replicated-sdk/pkg/store" ) -func TestSendCustomApplicationMetrics(t *testing.T) { +func TestSendCustomAppMetrics(t *testing.T) { // Happy path only channelSequence := int64(1) @@ -77,7 +77,7 @@ func TestSendCustomApplicationMetrics(t *testing.T) { defer store.SetStore(nil) if err := pact.Verify(func() error { - handlers.SendCustomApplicationMetrics(clientWriter, clientRequest) + handlers.SendCustomAppMetrics(clientWriter, clientRequest) if clientWriter.Code != http.StatusOK { return fmt.Errorf("expected status code %d but got %d", http.StatusOK, clientWriter.Code) } diff --git a/pkg/apiserver/server.go b/pkg/apiserver/server.go index 86c28af9..20a784ca 100644 --- a/pkg/apiserver/server.go +++ b/pkg/apiserver/server.go @@ -51,6 +51,10 @@ func Start(params APIServerParams) { r := mux.NewRouter() r.Use(handlers.CorsMiddleware) + // TODO: make all routes authenticated + authRouter := r.NewRoute().Subrouter() + authRouter.Use(handlers.RequireValidLicenseIDMiddleware) + r.HandleFunc("/healthz", handlers.Healthz) // license @@ -62,15 +66,14 @@ func Start(params APIServerParams) { r.HandleFunc("/api/v1/app/info", handlers.GetCurrentAppInfo).Methods("GET") r.HandleFunc("/api/v1/app/updates", handlers.GetAppUpdates).Methods("GET") r.HandleFunc("/api/v1/app/history", handlers.GetAppHistory).Methods("GET") + authRouter.HandleFunc("/api/v1/app/metrics", handlers.GetAppMetrics).Methods("GET") + r.HandleFunc("/api/v1/app/custom-metrics", handlers.SendCustomAppMetrics).Methods("POST") // integration r.HandleFunc("/api/v1/integration/mock-data", handlers.EnforceMockAccess(handlers.PostIntegrationMockData)).Methods("POST") r.HandleFunc("/api/v1/integration/mock-data", handlers.EnforceMockAccess(handlers.GetIntegrationMockData)).Methods("GET") r.HandleFunc("/api/v1/integration/status", handlers.EnforceMockAccess(handlers.GetIntegrationStatus)).Methods("GET") - // Custom metrics - r.HandleFunc("/api/v1/app/custom-metrics", handlers.SendCustomApplicationMetrics).Methods("POST") - srv := &http.Server{ Handler: r, Addr: ":3000", diff --git a/pkg/handlers/app.go b/pkg/handlers/app.go index 1cfde832..89eb0bc6 100644 --- a/pkg/handlers/app.go +++ b/pkg/handlers/app.go @@ -1,7 +1,9 @@ package handlers import ( + "encoding/json" "net/http" + "reflect" "sort" "strings" "time" @@ -9,12 +11,14 @@ import ( "github.com/pkg/errors" appstatetypes "github.com/replicatedhq/replicated-sdk/pkg/appstate/types" "github.com/replicatedhq/replicated-sdk/pkg/config" + "github.com/replicatedhq/replicated-sdk/pkg/heartbeat" "github.com/replicatedhq/replicated-sdk/pkg/helm" "github.com/replicatedhq/replicated-sdk/pkg/integration" integrationtypes "github.com/replicatedhq/replicated-sdk/pkg/integration/types" "github.com/replicatedhq/replicated-sdk/pkg/k8sutil" sdklicense "github.com/replicatedhq/replicated-sdk/pkg/license" "github.com/replicatedhq/replicated-sdk/pkg/logger" + "github.com/replicatedhq/replicated-sdk/pkg/metrics" "github.com/replicatedhq/replicated-sdk/pkg/store" "github.com/replicatedhq/replicated-sdk/pkg/upstream" upstreamtypes "github.com/replicatedhq/replicated-sdk/pkg/upstream/types" @@ -46,6 +50,12 @@ type AppRelease struct { HelmReleaseNamespace string `json:"helmReleaseNamespace,omitempty"` } +type SendCustomAppMetricsRequest struct { + Data CustomAppMetricsData `json:"data"` +} + +type CustomAppMetricsData map[string]interface{} + func GetCurrentAppInfo(w http.ResponseWriter, r *http.Request) { clientset, err := k8sutil.GetClientset() if err != nil { @@ -315,3 +325,65 @@ func mockReleaseToAppRelease(mockRelease integrationtypes.MockRelease) AppReleas return appRelease } + +func GetAppMetrics(w http.ResponseWriter, r *http.Request) { + heartbeatInfo := heartbeat.GetHeartbeatInfo(store.GetStore()) + headers := heartbeat.GetHeartbeatInfoHeaders(heartbeatInfo) + + JSON(w, http.StatusOK, headers) +} + +func SendCustomAppMetrics(w http.ResponseWriter, r *http.Request) { + license := store.GetStore().GetLicense() + + if util.IsAirgap() { + JSON(w, http.StatusForbidden, "This request cannot be satisfied in airgap mode") + return + } + + request := SendCustomAppMetricsRequest{} + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + logger.Error(errors.Wrap(err, "decode request")) + w.WriteHeader(http.StatusBadRequest) + return + } + + if err := validateCustomAppMetricsData(request.Data); err != nil { + w.WriteHeader(http.StatusBadRequest) + w.Write([]byte(err.Error())) + return + } + + err := metrics.SendCustomAppMetricsData(store.GetStore(), license, request.Data) + if err != nil { + logger.Error(errors.Wrap(err, "set application data")) + w.WriteHeader(http.StatusBadRequest) + return + } + + JSON(w, http.StatusOK, "") +} + +func validateCustomAppMetricsData(data CustomAppMetricsData) error { + if len(data) == 0 { + return errors.New("no data provided") + } + + for key, val := range data { + valType := reflect.TypeOf(val) + if valType == nil { + return errors.Errorf("%s value is nil, only scalar values are allowed", key) + } + + switch valType.Kind() { + case reflect.Slice: + return errors.Errorf("%s value is an array, only scalar values are allowed", key) + case reflect.Array: + return errors.Errorf("%s value is an array, only scalar values are allowed", key) + case reflect.Map: + return errors.Errorf("%s value is a map, only scalar values are allowed", key) + } + } + + return nil +} diff --git a/pkg/handlers/custom_metrics_test.go b/pkg/handlers/app_test.go similarity index 73% rename from pkg/handlers/custom_metrics_test.go rename to pkg/handlers/app_test.go index 6928a857..45d91115 100644 --- a/pkg/handlers/custom_metrics_test.go +++ b/pkg/handlers/app_test.go @@ -6,15 +6,15 @@ import ( "github.com/stretchr/testify/require" ) -func Test_validateCustomMetricsData(t *testing.T) { +func Test_validateCustomAppMetricsData(t *testing.T) { tests := []struct { name string - data ApplicationMetricsData + data CustomAppMetricsData wantErr bool }{ { name: "all values are valid", - data: ApplicationMetricsData{ + data: CustomAppMetricsData{ "key1": "val1", "key2": 6, "key3": 6.6, @@ -24,12 +24,12 @@ func Test_validateCustomMetricsData(t *testing.T) { }, { name: "no data", - data: ApplicationMetricsData{}, + data: CustomAppMetricsData{}, wantErr: true, }, { name: "array value", - data: ApplicationMetricsData{ + data: CustomAppMetricsData{ "key1": 10, "key2": []string{"val1", "val2"}, }, @@ -37,7 +37,7 @@ func Test_validateCustomMetricsData(t *testing.T) { }, { name: "map value", - data: ApplicationMetricsData{ + data: CustomAppMetricsData{ "key1": 10, "key2": map[string]string{"key1": "val1"}, }, @@ -45,7 +45,7 @@ func Test_validateCustomMetricsData(t *testing.T) { }, { name: "nil value", - data: ApplicationMetricsData{ + data: CustomAppMetricsData{ "key1": nil, "key2": 4, }, @@ -55,7 +55,7 @@ func Test_validateCustomMetricsData(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - err := validateCustomMetricsData(test.data) + err := validateCustomAppMetricsData(test.data) if test.wantErr { require.Error(t, err) } else { diff --git a/pkg/handlers/custom_metrics.go b/pkg/handlers/custom_metrics.go deleted file mode 100644 index 20a58789..00000000 --- a/pkg/handlers/custom_metrics.go +++ /dev/null @@ -1,74 +0,0 @@ -package handlers - -import ( - "encoding/json" - "net/http" - "reflect" - - "github.com/pkg/errors" - "github.com/replicatedhq/replicated-sdk/pkg/logger" - "github.com/replicatedhq/replicated-sdk/pkg/metrics" - "github.com/replicatedhq/replicated-sdk/pkg/store" - "github.com/replicatedhq/replicated-sdk/pkg/util" -) - -type SendCustomApplicationMetricsRequest struct { - Data ApplicationMetricsData `json:"data"` -} - -type ApplicationMetricsData map[string]interface{} - -func SendCustomApplicationMetrics(w http.ResponseWriter, r *http.Request) { - license := store.GetStore().GetLicense() - - if util.IsAirgap() { - JSON(w, http.StatusForbidden, "This request cannot be satisfied in airgap mode") - return - } - - request := SendCustomApplicationMetricsRequest{} - if err := json.NewDecoder(r.Body).Decode(&request); err != nil { - logger.Error(errors.Wrap(err, "decode request")) - w.WriteHeader(http.StatusBadRequest) - return - } - - if err := validateCustomMetricsData(request.Data); err != nil { - w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) - return - } - - err := metrics.SendApplicationMetricsData(store.GetStore(), license, request.Data) - if err != nil { - logger.Error(errors.Wrap(err, "set application data")) - w.WriteHeader(http.StatusBadRequest) - return - } - - JSON(w, http.StatusOK, "") -} - -func validateCustomMetricsData(data ApplicationMetricsData) error { - if len(data) == 0 { - return errors.New("no data provided") - } - - for key, val := range data { - valType := reflect.TypeOf(val) - if valType == nil { - return errors.Errorf("%s value is nil, only scalar values are allowed", key) - } - - switch valType.Kind() { - case reflect.Slice: - return errors.Errorf("%s value is an array, only scalar values are allowed", key) - case reflect.Array: - return errors.Errorf("%s value is an array, only scalar values are allowed", key) - case reflect.Map: - return errors.Errorf("%s value is a map, only scalar values are allowed", key) - } - } - - return nil -} diff --git a/pkg/handlers/middleware.go b/pkg/handlers/middleware.go index c2405d45..93739014 100644 --- a/pkg/handlers/middleware.go +++ b/pkg/handlers/middleware.go @@ -3,6 +3,7 @@ package handlers import ( "net/http" + "github.com/replicatedhq/replicated-sdk/pkg/handlers/types" "github.com/replicatedhq/replicated-sdk/pkg/store" ) @@ -24,3 +25,22 @@ func EnforceMockAccess(next http.HandlerFunc) http.HandlerFunc { next.ServeHTTP(w, r) } } + +func RequireValidLicenseIDMiddleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + licenseID := r.Header.Get("authorization") + if licenseID == "" { + response := types.ErrorResponse{Error: "missing authorization header"} + JSON(w, http.StatusUnauthorized, response) + return + } + + if store.GetStore().GetLicense().Spec.LicenseID != licenseID { + response := types.ErrorResponse{Error: "license ID is not valid"} + JSON(w, http.StatusUnauthorized, response) + return + } + + next.ServeHTTP(w, r) + }) +} diff --git a/pkg/handlers/types/types.go b/pkg/handlers/types/types.go new file mode 100644 index 00000000..9f60069f --- /dev/null +++ b/pkg/handlers/types/types.go @@ -0,0 +1,6 @@ +package types + +type ErrorResponse struct { + Error string `json:"error,omitempty"` + Success bool `json:"success"` +} diff --git a/pkg/heartbeat/util.go b/pkg/heartbeat/util.go index cf328cf9..242fcd39 100644 --- a/pkg/heartbeat/util.go +++ b/pkg/heartbeat/util.go @@ -16,26 +16,38 @@ import ( ) func InjectHeartbeatInfoHeaders(req *http.Request, heartbeatInfo *types.HeartbeatInfo) { + headers := GetHeartbeatInfoHeaders(heartbeatInfo) + + for key, value := range headers { + req.Header.Set(key, value) + } +} + +func GetHeartbeatInfoHeaders(heartbeatInfo *types.HeartbeatInfo) map[string]string { + headers := make(map[string]string) + if heartbeatInfo == nil { - return + return headers } - req.Header.Set("X-Replicated-K8sVersion", heartbeatInfo.K8sVersion) - req.Header.Set("X-Replicated-AppStatus", heartbeatInfo.AppStatus) - req.Header.Set("X-Replicated-ClusterID", heartbeatInfo.ClusterID) - req.Header.Set("X-Replicated-InstanceID", heartbeatInfo.InstanceID) + headers["X-Replicated-K8sVersion"] = heartbeatInfo.K8sVersion + headers["X-Replicated-AppStatus"] = heartbeatInfo.AppStatus + headers["X-Replicated-ClusterID"] = heartbeatInfo.ClusterID + headers["X-Replicated-InstanceID"] = heartbeatInfo.InstanceID if heartbeatInfo.ChannelID != "" { - req.Header.Set("X-Replicated-DownstreamChannelID", heartbeatInfo.ChannelID) + headers["X-Replicated-DownstreamChannelID"] = heartbeatInfo.ChannelID } else if heartbeatInfo.ChannelName != "" { - req.Header.Set("X-Replicated-DownstreamChannelName", heartbeatInfo.ChannelName) + headers["X-Replicated-DownstreamChannelName"] = heartbeatInfo.ChannelName } - req.Header.Set("X-Replicated-DownstreamChannelSequence", strconv.FormatInt(heartbeatInfo.ChannelSequence, 10)) + headers["X-Replicated-DownstreamChannelSequence"] = strconv.FormatInt(heartbeatInfo.ChannelSequence, 10) if heartbeatInfo.K8sDistribution != "" { - req.Header.Set("X-Replicated-K8sDistribution", heartbeatInfo.K8sDistribution) + headers["X-Replicated-K8sDistribution"] = heartbeatInfo.K8sDistribution } + + return headers } func canReport(clientset kubernetes.Interface, namespace string, license *kotsv1beta1.License) (bool, error) { diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index ec4397aa..d2a4de97 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -15,7 +15,7 @@ import ( "github.com/replicatedhq/replicated-sdk/pkg/util" ) -func SendApplicationMetricsData(sdkStore store.Store, license *kotsv1beta1.License, data map[string]interface{}) error { +func SendCustomAppMetricsData(sdkStore store.Store, license *kotsv1beta1.License, data map[string]interface{}) error { endpoint := sdkStore.GetReplicatedAppEndpoint() if endpoint == "" { endpoint = license.Spec.Endpoint From 976793e575bceac3ab2cd8992952692bbe86656b Mon Sep 17 00:00:00 2001 From: Salah Aldeen Al Saleh Date: Tue, 10 Oct 2023 16:35:09 +0000 Subject: [PATCH 2/4] add tests --- .github/actions/validate-endpoints/action.yml | 10 ++++++ pkg/handlers/types/types.go | 3 +- pkg/heartbeat/util_test.go | 31 +++++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/.github/actions/validate-endpoints/action.yml b/.github/actions/validate-endpoints/action.yml index b402c4ff..71b2b187 100644 --- a/.github/actions/validate-endpoints/action.yml +++ b/.github/actions/validate-endpoints/action.yml @@ -142,3 +142,13 @@ runs: exit 1 fi fi + + - name: Validate /app/metrics endpoint + shell: bash + run: | + appStatusMetric=$(curl -s --fail --show-error localhost:8888/api/v1/app/metrics | jq '."X-Replicated-AppStatus"' | tr -d '\n') + + if [ "$appStatusMetric" != "ready" ]; then + echo "Expected app status metric 'X-Replicated-AppStatus' to be 'ready', but is set to '$appStatusMetric'." + exit 1 + fi diff --git a/pkg/handlers/types/types.go b/pkg/handlers/types/types.go index 9f60069f..2100bac6 100644 --- a/pkg/handlers/types/types.go +++ b/pkg/handlers/types/types.go @@ -1,6 +1,5 @@ package types type ErrorResponse struct { - Error string `json:"error,omitempty"` - Success bool `json:"success"` + Error string `json:"error,omitempty"` } diff --git a/pkg/heartbeat/util_test.go b/pkg/heartbeat/util_test.go index b600d760..a74bae4d 100644 --- a/pkg/heartbeat/util_test.go +++ b/pkg/heartbeat/util_test.go @@ -3,11 +3,42 @@ package heartbeat import ( "testing" + "github.com/replicatedhq/replicated-sdk/pkg/heartbeat/types" "github.com/replicatedhq/replicated-sdk/pkg/k8sutil" "github.com/replicatedhq/replicated-sdk/pkg/util" + "github.com/stretchr/testify/assert" "k8s.io/client-go/kubernetes/fake" ) +func TestGetHeartbeatInfoHeaders(t *testing.T) { + heartbeatInfo := &types.HeartbeatInfo{ + AppStatus: "ready", + ClusterID: "cluster-123", + InstanceID: "instance-456", + ChannelID: "channel-789", + ChannelSequence: 42, + K8sVersion: "v1.20.2+k3s1", + K8sDistribution: "k3s", + } + + headers := GetHeartbeatInfoHeaders(heartbeatInfo) + + expectedHeaders := map[string]string{ + "X-Replicated-K8sVersion": "v1.20.2+k3s1", + "X-Replicated-AppStatus": "ready", + "X-Replicated-ClusterID": "cluster-123", + "X-Replicated-InstanceID": "instance-456", + "X-Replicated-DownstreamChannelID": "channel-789", + "X-Replicated-DownstreamChannelSequence": "42", + "X-Replicated-K8sDistribution": "k3s", + } + + assert.Equal(t, expectedHeaders, headers) + + nilHeaders := GetHeartbeatInfoHeaders(nil) + assert.Empty(t, nilHeaders) +} + func TestCanReport(t *testing.T) { tests := []struct { name string From 06c819c66640693e412b94bba5e7f0fe37f3d815 Mon Sep 17 00:00:00 2001 From: Salah Aldeen Al Saleh Date: Tue, 10 Oct 2023 16:53:34 +0000 Subject: [PATCH 3/4] use license ID as auth for test --- .github/actions/validate-endpoints/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/validate-endpoints/action.yml b/.github/actions/validate-endpoints/action.yml index 71b2b187..5160a2b9 100644 --- a/.github/actions/validate-endpoints/action.yml +++ b/.github/actions/validate-endpoints/action.yml @@ -146,7 +146,7 @@ runs: - name: Validate /app/metrics endpoint shell: bash run: | - appStatusMetric=$(curl -s --fail --show-error localhost:8888/api/v1/app/metrics | jq '."X-Replicated-AppStatus"' | tr -d '\n') + appStatusMetric=$(curl -H 'Authorization: ${{ inputs.license-id }}' -s --fail --show-error localhost:8888/api/v1/app/metrics | jq '."X-Replicated-AppStatus"' | tr -d '\n') if [ "$appStatusMetric" != "ready" ]; then echo "Expected app status metric 'X-Replicated-AppStatus' to be 'ready', but is set to '$appStatusMetric'." From 4044408bac9f6430d4b7ce0cc4aa75914a180238 Mon Sep 17 00:00:00 2001 From: Salah Aldeen Al Saleh Date: Tue, 10 Oct 2023 17:01:41 +0000 Subject: [PATCH 4/4] strip quotes --- .github/actions/validate-endpoints/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/validate-endpoints/action.yml b/.github/actions/validate-endpoints/action.yml index 5160a2b9..0e205eea 100644 --- a/.github/actions/validate-endpoints/action.yml +++ b/.github/actions/validate-endpoints/action.yml @@ -146,7 +146,7 @@ runs: - name: Validate /app/metrics endpoint shell: bash run: | - appStatusMetric=$(curl -H 'Authorization: ${{ inputs.license-id }}' -s --fail --show-error localhost:8888/api/v1/app/metrics | jq '."X-Replicated-AppStatus"' | tr -d '\n') + appStatusMetric=$(curl -H 'Authorization: ${{ inputs.license-id }}' -s --fail --show-error localhost:8888/api/v1/app/metrics | jq -r '."X-Replicated-AppStatus"' | tr -d '\n') if [ "$appStatusMetric" != "ready" ]; then echo "Expected app status metric 'X-Replicated-AppStatus' to be 'ready', but is set to '$appStatusMetric'."