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(metrics-operator): provide more information for dynatrace api error #2885

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
29ce035
fix: delete error message after successful retrieval of value
Vickysomtee Jan 3, 2024
2f9d9a2
Merge branch 'main' of https://github.com/Vickysomtee/keptn-lifecycle…
Vickysomtee Jan 3, 2024
6061ec3
test: successfully delete error message
Vickysomtee Jan 15, 2024
4d00167
Merge branch 'main' of https://github.com/Vickysomtee/keptn-lifecycle…
Vickysomtee Jan 15, 2024
02e2ed2
test: successfully delete error message
Vickysomtee Jan 15, 2024
6828600
Merge branch 'main' of https://github.com/Vickysomtee/keptn-lifecycle…
Vickysomtee Jan 26, 2024
2539fcb
fix: improve dynatrace api error message
Vickysomtee Jan 26, 2024
039c504
Merge branch 'main' of https://github.com/Vickysomtee/keptn-lifecycle…
Vickysomtee Feb 1, 2024
69854e0
test: API Client Auth Response
Vickysomtee Feb 1, 2024
3c25460
Merge branch 'main' of https://github.com/Vickysomtee/keptn-lifecycle…
Vickysomtee Feb 19, 2024
81a5858
test: chainsaw test
Vickysomtee Feb 20, 2024
af12ae3
test: chainsaw test
Vickysomtee Feb 21, 2024
1a82cee
test: chainsaw test
Vickysomtee Feb 22, 2024
9025ec4
test: chainsaw test
Vickysomtee Feb 23, 2024
86ea1b2
test: chainsaw test
Vickysomtee Feb 28, 2024
65883df
test: chainsaw test
Vickysomtee Feb 29, 2024
941b602
test: chainsaw test
Vickysomtee Feb 29, 2024
ae5a4fa
fix: PR conflict
Vickysomtee Mar 10, 2024
2ff007a
Merge branch 'main' of https://github.com/Vickysomtee/keptn-lifecycle…
Vickysomtee Apr 3, 2024
129c058
test: fixed failing unit test
Vickysomtee Apr 19, 2024
2794b98
test: update mock server http request
Vickysomtee Apr 23, 2024
f4b4a64
refactor: header request
Vickysomtee Apr 29, 2024
c169a3c
Merge branch 'main' of https://github.com/Vickysomtee/keptn-lifecycle…
Vickysomtee Apr 29, 2024
8d5daf0
Merge branch 'main' of https://github.com/Vickysomtee/keptn-lifecycle…
Vickysomtee May 7, 2024
131675f
refactor: header request
Vickysomtee May 7, 2024
b34e7ab
refactor: header request
Vickysomtee May 7, 2024
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
Vickysomtee marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ func NewAPIClient(config apiConfig, options ...APIClientOption) *apiClient {

// Do sends and API request to the Dynatrace API and returns its result as a string containing the raw response payload
func (client *apiClient) Do(ctx context.Context, path, method string, payload []byte) ([]byte, int, error) {
if err := client.auth(ctx); err != nil {
return nil, http.StatusUnauthorized, err
if statusCode, err := client.auth(ctx); err != nil {
return nil, statusCode, err
}
api := fmt.Sprintf("%s%s", client.config.serverURL, path)
req, err := http.NewRequestWithContext(ctx, method, api, bytes.NewBuffer(payload))
Expand Down Expand Up @@ -89,10 +89,10 @@ func (client *apiClient) Do(ctx context.Context, path, method string, payload []
return b, res.StatusCode, nil
}

func (client *apiClient) auth(ctx context.Context) error {
func (client *apiClient) auth(ctx context.Context) (int, error) {
odubajDT marked this conversation as resolved.
Show resolved Hide resolved
// return if we already have a token
if client.config.oAuthCredentials.accessToken != "" {
return nil
return http.StatusOK, nil
}
client.Log.V(10).Info("OAuth login")
ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
Expand All @@ -103,12 +103,12 @@ func (client *apiClient) auth(ctx context.Context) error {

req, err := http.NewRequestWithContext(ctx, "POST", client.config.authURL, bytes.NewBuffer(body))
if err != nil {
return err
return http.StatusInternalServerError, err
}
req.Header.Add("Content-Type", "application/x-www-form-urlencoded")
res, err := client.httpClient.Do(req)
if err != nil {
return err
return http.StatusInternalServerError, err
}
defer func() {
err := res.Body.Close()
Expand All @@ -117,20 +117,20 @@ func (client *apiClient) auth(ctx context.Context) error {
}
}()
if isErrorStatus(res.StatusCode) {
return ErrRequestFailed
return res.StatusCode, ErrRequestFailed
}
// we ignore the error here because we fail later while unmarshalling
oauthResponse := OAuthResponse{}
b, _ := io.ReadAll(res.Body)
err = json.Unmarshal(b, &oauthResponse)
if err != nil {
return err
return http.StatusInternalServerError, err
}

if oauthResponse.AccessToken == "" {
return ErrAuthenticationFailed
return http.StatusUnauthorized, ErrAuthenticationFailed
}

client.config.oAuthCredentials.accessToken = oauthResponse.AccessToken
return nil
return http.StatusOK, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func TestAPIClientAuthError(t *testing.T) {

require.ErrorIs(t, err, ErrRequestFailed)
require.Empty(t, resp)
require.Equal(t, http.StatusUnauthorized, code)
require.Equal(t, http.StatusInternalServerError, code)
}

func TestAPIClientAuthNoToken(t *testing.T) {
Expand Down Expand Up @@ -154,3 +154,39 @@ func TestAPIClientRequestError(t *testing.T) {
require.Empty(t, resp)
require.Equal(t, http.StatusInternalServerError, code)
}

func TestAPIClientAuthResponse(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
if request.URL.Path == "/auth" {
_, _ = writer.Write([]byte(`{"access_token": "my-token"}`))
return
}
writer.WriteHeader(http.StatusInternalServerError)
}))

defer server.Close()

config, err := NewAPIConfig(
server.URL,
mockSecret,
WithAuthURL(server.URL+"/auth"),
)

require.Nil(t, err)
require.NotNil(t, config)

apiClient := NewAPIClient(
*config,
WithHTTPClient(http.Client{}),
WithLogger(logr.New(klog.NewKlogr().GetSink())),
)

require.NotNil(t, apiClient)

int, err := apiClient.auth(context.TODO())

require.Equal(t, int, http.StatusOK)
require.Equal(t, err, nil)
RealAnna marked this conversation as resolved.
Show resolved Hide resolved

require.Equal(t, "my-token", apiClient.config.oAuthCredentials.accessToken)
}
8 changes: 8 additions & 0 deletions test/chainsaw/testmetrics/dynatrace-provider/00-assert.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: metrics.keptn.sh/v1beta1
Vickysomtee marked this conversation as resolved.
Show resolved Hide resolved
kind: KeptnMetric
metadata:
name: podtatometric
labels:
app: podtato-head
status:
Copy link
Contributor

Choose a reason for hiding this comment

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

you are getting an empty status in the test result. I would say that's not expected. It should either return the value or error. Can you please adapt it accordingly?

errMsg: ""
odubajDT marked this conversation as resolved.
Show resolved Hide resolved
30 changes: 30 additions & 0 deletions test/chainsaw/testmetrics/dynatrace-provider/00-install.yaml
Vickysomtee marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
apiVersion: metrics.keptn.sh/v1beta1
kind: KeptnMetric
metadata:
name: podtatometric
spec:
provider:
name: "dynatrace"
query: "query"
fetchIntervalSeconds: 5

---
apiVersion: metrics.keptn.sh/v1beta1
kind: KeptnMetricsProvider
metadata:
name: dynatrace
spec:
secretKeyRef:
RealAnna marked this conversation as resolved.
Show resolved Hide resolved
key: DT_TOKEN
name: dynatrace
type: dynatrace
targetServer: (join('.', ['http://mockserver', $namespace, 'svc.cluster.local:1080']))

---
apiVersion: v1
kind: Secret
metadata:
name: dynatrace
type: Opaque
data:
DT_TOKEN: dG9rZW46IG15dG9rZW4=
19 changes: 19 additions & 0 deletions test/chainsaw/testmetrics/dynatrace-provider/chainsaw-test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json
apiVersion: chainsaw.kyverno.io/v1alpha1
kind: Test
metadata:
name: dynatrace-provider-auth
spec:
namespaceTemplate:
metadata:
annotations:
keptn.sh/lifecycle-toolkit: enabled
steps:
- name: step-00
try:
- apply:
file: mock-server.yaml
- apply:
file: 00-install.yaml
Vickysomtee marked this conversation as resolved.
Show resolved Hide resolved
- assert:
file: 00-assert.yaml
142 changes: 142 additions & 0 deletions test/chainsaw/testmetrics/dynatrace-provider/mock-server.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
apiVersion: v1
kind: Service
metadata:
name: mockserver
spec:
ports:
- name: serviceport
port: 1080
protocol: TCP
targetPort: serviceport
selector:
app: mockserver
sessionAffinity: None
type: ClusterIP
---
apiVersion: apps/v1
kind: Deployment
metadata:
labels:
app: mockserver
name: mockserver
spec:
replicas: 1
selector:
matchLabels:
app: mockserver
template:
metadata:
labels:
app: mockserver
name: mockserver
spec:
automountServiceAccountToken: false
containers:
- env:
- name: MOCKSERVER_LOG_LEVEL
value: INFO
- name: SERVER_PORT
value: "1080"
image: mockserver/mockserver:mockserver-5.13.0
imagePullPolicy: IfNotPresent
resources:
limits:
cpu: 500m
memory: 128Mi
requests:
cpu: 10m
memory: 64Mi
livenessProbe:
failureThreshold: 10
initialDelaySeconds: 10
periodSeconds: 5
successThreshold: 1
tcpSocket:
port: serviceport
timeoutSeconds: 1
name: mockserver
ports:
- containerPort: 1080
name: serviceport
protocol: TCP
readinessProbe:
failureThreshold: 10
initialDelaySeconds: 2
periodSeconds: 2
successThreshold: 1
tcpSocket:
port: serviceport
timeoutSeconds: 1
volumeMounts:
- mountPath: /config
name: config-volume
- mountPath: /libs
name: libs-volume
terminationGracePeriodSeconds: 30
volumes:
- configMap:
defaultMode: 420
name: mockserver-config
optional: true
name: config-volume
- configMap:
defaultMode: 420
name: mockserver-config
optional: true
name: libs-volume
---
kind: ConfigMap
apiVersion: v1
metadata:
name: mockserver-config
data:
initializerJson.json: |-
[
{
"httpRequest": {
"method" : "GET",
"path" : "/api/v2/metrics/query",
"queryStringParameters" : {
"metricSelector" : [ "query" ]
}
},
"httpResponse": {
"body": {
"status": "error",
"data": {
"resultType": "matrix",
"result": []
}
},
"statusCode": 401
}
}
]
mockserver.properties: |-
###############################
# MockServer & Proxy Settings #
###############################
# Socket & Port Settings
# socket timeout in milliseconds (default 120000)
mockserver.maxSocketTimeout=120000
# Certificate Generation
# dynamically generated CA key pair (if they don't already exist in
specified directory)
mockserver.dynamicallyCreateCertificateAuthorityCertificate=true
# save dynamically generated CA key pair in working directory
mockserver.directoryToSaveDynamicSSLCertificate=.
# certificate domain name (default "localhost")
mockserver.sslCertificateDomainName=localhost
# comma separated list of ip addresses for Subject Alternative Name domain
names (default empty list)
mockserver.sslSubjectAlternativeNameDomains=www.example.com,www.another.com
# comma separated list of ip addresses for Subject Alternative Name ips
(default empty list)
mockserver.sslSubjectAlternativeNameIps=127.0.0.1
# CORS
# enable CORS for MockServer REST API
mockserver.enableCORSForAPI=true
# enable CORS for all responses
mockserver.enableCORSForAllResponses=true
# Json Initialization
mockserver.initializationJsonPath=/config/initializerJson.json
Loading