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

fix(metricprovider): Support jsonBody for web metric provider Fixes #2275 #2312

Merged
merged 2 commits into from
Oct 19, 2022
Merged
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
31 changes: 27 additions & 4 deletions docs/analysis/web.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ to convert a result value to a numeric type so that mathematical comparison oper
(e.g. >, <, >=, <=).

### Optional web methods
It is possible to use a POST or PUT requests, by specifying the `method` and `body` fields
It is possible to use a POST or PUT requests, by specifying the `method` and either `body` or `jsonBody` fields

```yaml
metrics:
Expand All @@ -66,9 +66,32 @@ It is possible to use a POST or PUT requests, by specifying the `method` and `bo
value: "Bearer {{ args.api-token }}"
- key: Content-Type # if body is a json, it is recommended to set the Content-Type
value: "application/json"
body: "{\"key\": \"string value\"}"
body: "string value"
jsonPath: "{$.data.ok}"
```
!!! tip
In order to send in JSON, you have to encode it yourself, and send the correct Content-Type as well.
Setting a `body` field for a `GET` request will result in an error.
In order to send in JSON, you can use jsonBody and Content-Type will be automatically set as json.
Setting a `body` or `jsonBody` field for a `GET` request will result in an error.
Set either `body` or `jsonBody` and setting both will result in an error.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mayzhang2000 Setting both will result in an error. Should I be removing this line from the doc?


```yaml
metrics:
- name: webmetric
successCondition: result == true
provider:
web:
method: POST # valid values are GET|POST|PUT, defaults to GET
url: "http://my-server.com/api/v1/measurement?service={{ args.service-name }}"
timeoutSeconds: 20 # defaults to 10 seconds
headers:
- key: Authorization
value: "Bearer {{ args.api-token }}"
- key: Content-Type # if body is a json, it is recommended to set the Content-Type
value: "application/json"
jsonBody: # If using jsonBody Content-Type header will be automatically set to json
key1: value_1
key2:
nestedObj: nested value
key3: "{{ args.service-name }}"
jsonPath: "{$.data.ok}"
```
12 changes: 12 additions & 0 deletions docs/features/kustomize/rollout_cr_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -4213,6 +4213,10 @@
"insecure": {
"type": "boolean"
},
"jsonBody": {
"type": "object",
"x-kubernetes-preserve-unknown-fields": true
},
"jsonPath": {
"type": "string"
},
Expand Down Expand Up @@ -8477,6 +8481,10 @@
"insecure": {
"type": "boolean"
},
"jsonBody": {
"type": "object",
"x-kubernetes-preserve-unknown-fields": true
},
"jsonPath": {
"type": "string"
},
Expand Down Expand Up @@ -12741,6 +12749,10 @@
"insecure": {
"type": "boolean"
},
"jsonBody": {
"type": "object",
"x-kubernetes-preserve-unknown-fields": true
},
"jsonPath": {
"type": "string"
},
Expand Down
3 changes: 3 additions & 0 deletions manifests/crds/analysis-run-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2732,6 +2732,9 @@ spec:
type: array
insecure:
type: boolean
jsonBody:
type: object
x-kubernetes-preserve-unknown-fields: true
jsonPath:
type: string
method:
Expand Down
3 changes: 3 additions & 0 deletions manifests/crds/analysis-template-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2728,6 +2728,9 @@ spec:
type: array
insecure:
type: boolean
jsonBody:
type: object
x-kubernetes-preserve-unknown-fields: true
jsonPath:
type: string
method:
Expand Down
3 changes: 3 additions & 0 deletions manifests/crds/cluster-analysis-template-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2728,6 +2728,9 @@ spec:
type: array
insecure:
type: boolean
jsonBody:
type: object
x-kubernetes-preserve-unknown-fields: true
jsonPath:
type: string
method:
Expand Down
9 changes: 9 additions & 0 deletions manifests/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2733,6 +2733,9 @@ spec:
type: array
insecure:
type: boolean
jsonBody:
type: object
x-kubernetes-preserve-unknown-fields: true
jsonPath:
type: string
method:
Expand Down Expand Up @@ -5607,6 +5610,9 @@ spec:
type: array
insecure:
type: boolean
jsonBody:
type: object
x-kubernetes-preserve-unknown-fields: true
jsonPath:
type: string
method:
Expand Down Expand Up @@ -8367,6 +8373,9 @@ spec:
type: array
insecure:
type: boolean
jsonBody:
type: object
x-kubernetes-preserve-unknown-fields: true
jsonPath:
type: string
method:
Expand Down
9 changes: 9 additions & 0 deletions manifests/namespace-install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2733,6 +2733,9 @@ spec:
type: array
insecure:
type: boolean
jsonBody:
type: object
x-kubernetes-preserve-unknown-fields: true
jsonPath:
type: string
method:
Expand Down Expand Up @@ -5607,6 +5610,9 @@ spec:
type: array
insecure:
type: boolean
jsonBody:
type: object
x-kubernetes-preserve-unknown-fields: true
jsonPath:
type: string
method:
Expand Down Expand Up @@ -8367,6 +8373,9 @@ spec:
type: array
insecure:
type: boolean
jsonBody:
type: object
x-kubernetes-preserve-unknown-fields: true
jsonPath:
type: string
method:
Expand Down
29 changes: 23 additions & 6 deletions metricproviders/webmetric/webmetric.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package webmetric

import (
"bytes"
"crypto/tls"
"encoding/json"
"errors"
Expand All @@ -23,7 +24,9 @@ import (

const (
// ProviderType indicates the provider is a web metric
ProviderType = "Web"
ProviderType = "Web"
ContentTypeKey = "Content-Type"
ContentTypeJsonValue = "application/json"
)

// Provider contains all the required components to run a WebMetric query
Expand Down Expand Up @@ -59,14 +62,25 @@ func (p *Provider) Run(run *v1alpha1.AnalysisRun, metric v1alpha1.Metric) v1alph

url := metric.Provider.Web.URL

stringBody := metric.Provider.Web.Body
jsonBody := metric.Provider.Web.JSONBody

var body io.Reader

if metric.Provider.Web.Body != "" {
if method == v1alpha1.WebMetricMethodGet {
return metricutil.MarkMeasurementError(measurement, fmt.Errorf("Body can only be used with POST or PUT WebMetric Method types"))
}
if stringBody != "" && jsonBody != nil {
return metricutil.MarkMeasurementError(measurement, fmt.Errorf("use either Body or JSONBody; both cannot exists for WebMetric payload"))
} else if (stringBody != "" || jsonBody != nil) && method == v1alpha1.WebMetricMethodGet {
return metricutil.MarkMeasurementError(measurement, fmt.Errorf("Body/JSONBody can only be used with POST or PUT WebMetric Method types"))
}

body = strings.NewReader(metric.Provider.Web.Body)
if stringBody != "" {
body = strings.NewReader(stringBody)
} else if jsonBody != nil {
bodyBytes, err := jsonBody.MarshalJSON()
if err != nil {
return metricutil.MarkMeasurementError(measurement, err)
}
body = bytes.NewBuffer(bodyBytes)
}

// Create request
Expand All @@ -80,6 +94,9 @@ func (p *Provider) Run(run *v1alpha1.AnalysisRun, metric v1alpha1.Metric) v1alph
for _, header := range metric.Provider.Web.Headers {
request.Header.Set(header.Key, header.Value)
}
if jsonBody != nil {
request.Header.Set(ContentTypeKey, ContentTypeJsonValue)
}

// Send Request
response, err := p.client.Do(request)
Expand Down
83 changes: 83 additions & 0 deletions metricproviders/webmetric/webmetric_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package webmetric

import (
"bytes"
"encoding/json"
"io"
"net/http"
"net/http/httptest"
Expand All @@ -23,6 +24,7 @@ func TestRunSuite(t *testing.T) {
expectedValue string
expectedPhase v1alpha1.AnalysisPhase
expectedErrorMessage string
expectedJsonBody string
}{
// When_noJSONPathSpecified_And_MatchesConditions_Then_Succeed
{
Expand Down Expand Up @@ -574,6 +576,81 @@ func TestRunSuite(t *testing.T) {
expectedValue: "Body can only be used with POST or PUT WebMetric Method types",
expectedPhase: v1alpha1.AnalysisPhaseError,
},
// When_methodPOST_Then_server_gets_jsonBody_Then_Succeed
{
webServerStatus: 200,
webServerResponse: `{"a": 1, "b": true, "c": [1, 2, 3, 4], "d": null}`,
metric: v1alpha1.Metric{
Name: "foo",
SuccessCondition: "result.a > 0 && result.b && all(result.c, {# < 5}) && result.d == nil",
Provider: v1alpha1.MetricProvider{
Web: &v1alpha1.WebMetric{
Method: v1alpha1.WebMetricMethodPost,
// URL: server.URL,
Headers: []v1alpha1.WebMetricHeader{{Key: "key", Value: "value"}},
JSONBody: json.RawMessage(`{"key1": "value1", "key2": "value2"}`),
},
},
},
expectedMethod: "POST",
expectedJsonBody: `{"key1": "value1", "key2": "value2"}`,
expectedValue: `{"a":1,"b":true,"c":[1,2,3,4],"d":null}`,
expectedPhase: v1alpha1.AnalysisPhaseSuccessful,
},
// When_methodPUT_Then_server_gets_jsonBody_Then_Succeed
{
webServerStatus: 200,
webServerResponse: `{"a": 1, "b": true, "c": [1, 2, 3, 4], "d": null}`,
metric: v1alpha1.Metric{
Name: "foo",
SuccessCondition: "result.a > 0 && result.b && all(result.c, {# < 5}) && result.d == nil",
Provider: v1alpha1.MetricProvider{
Web: &v1alpha1.WebMetric{
Method: v1alpha1.WebMetricMethodPut,
// URL: server.URL,
Headers: []v1alpha1.WebMetricHeader{{Key: "key", Value: "value"}, {Key: ContentTypeKey, Value: ContentTypeJsonValue}},
JSONBody: json.RawMessage(`{"key1": "value1", "key2": { "key3" : "value3"}}`),
},
},
},
expectedMethod: "PUT",
expectedJsonBody: `{"key1": "value1", "key2": { "key3" : "value3"}}`,
expectedValue: `{"a":1,"b":true,"c":[1,2,3,4],"d":null}`,
expectedPhase: v1alpha1.AnalysisPhaseSuccessful,
},
// When_sendingJsonBodyWithGet_Then_Failure
{
metric: v1alpha1.Metric{
Name: "foo",
SuccessCondition: "result.a > 0 && result.b && all(result.c, {# < 5}) && result.d == nil",
Provider: v1alpha1.MetricProvider{
Web: &v1alpha1.WebMetric{
// URL: server.URL,
Headers: []v1alpha1.WebMetricHeader{{Key: "key", Value: "value"}},
JSONBody: json.RawMessage(`{"key1": "value1", "key2": { "key3" : "value3"}}`),
},
},
},
expectedValue: "Body/JSONBody can only be used with POST or PUT WebMetric Method types",
expectedPhase: v1alpha1.AnalysisPhaseError,
},
// When_sending_BothBodyAndJsonBodyWithGet_Then_Failure
{
metric: v1alpha1.Metric{
Name: "foo",
SuccessCondition: "result.a > 0 && result.b && all(result.c, {# < 5}) && result.d == nil",
Provider: v1alpha1.MetricProvider{
Web: &v1alpha1.WebMetric{
// URL: server.URL,
Headers: []v1alpha1.WebMetricHeader{{Key: "key", Value: "value"}},
JSONBody: json.RawMessage(`{"key1": "value1", "key2": { "key3" : "value3"}}`),
Body: "test body",
},
},
},
expectedValue: "use either Body or JSONBody; both cannot exists for WebMetric payload",
expectedPhase: v1alpha1.AnalysisPhaseError,
},
}

// Run
Expand All @@ -591,6 +668,12 @@ func TestRunSuite(t *testing.T) {
assert.Equal(t, test.expectedBody, buf.String())
}

if test.expectedJsonBody != "" {
bodyBytes, _ := io.ReadAll(req.Body)
assert.Equal(t, test.expectedJsonBody, string(bodyBytes))
assert.Equal(t, ContentTypeJsonValue, req.Header.Get(ContentTypeKey))
}

if test.webServerStatus < 200 || test.webServerStatus >= 300 {
http.Error(rw, http.StatusText(test.webServerStatus), test.webServerStatus)
} else {
Expand Down
8 changes: 7 additions & 1 deletion pkg/apis/rollouts/v1alpha1/analysis_types.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package v1alpha1

import (
"encoding/json"
"time"

intstrutil "k8s.io/apimachinery/pkg/util/intstr"
Expand Down Expand Up @@ -477,14 +478,19 @@ type WebMetric struct {
// +patchStrategy=merge
// Headers are optional HTTP headers to use in the request
Headers []WebMetricHeader `json:"headers,omitempty" patchStrategy:"merge" patchMergeKey:"key" protobuf:"bytes,3,rep,name=headers"`
// Body is the body of the we metric (must be POST/PUT)
// Body is the body of the web metric (must be POST/PUT)
Body string `json:"body,omitempty" protobuf:"bytes,4,opt,name=body"`
// TimeoutSeconds is the timeout for the request in seconds (default: 10)
TimeoutSeconds int64 `json:"timeoutSeconds,omitempty" protobuf:"varint,5,opt,name=timeoutSeconds"`
// JSONPath is a JSON Path to use as the result variable (default: "{$}")
JSONPath string `json:"jsonPath,omitempty" protobuf:"bytes,6,opt,name=jsonPath"`
// Insecure skips host TLS verification
Insecure bool `json:"insecure,omitempty" protobuf:"varint,7,opt,name=insecure"`
// +kubebuilder:validation:Schemaless
// +kubebuilder:pruning:PreserveUnknownFields
// +kubebuilder:validation:Type=object
// JSONBody is the body of the web metric in a json format (method must be POST/PUT)
JSONBody json.RawMessage `json:"jsonBody,omitempty" protobuf:"bytes,8,opt,name=jsonBody,casttype=encoding/json.RawMessage"`
}

// WebMetricMethod is the available HTTP methods
Expand Down
Loading