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

DELETE retried multiple times #60

Closed
irizzant opened this issue Oct 1, 2024 · 8 comments
Closed

DELETE retried multiple times #60

irizzant opened this issue Oct 1, 2024 · 8 comments
Labels
bug Something isn't working

Comments

@irizzant
Copy link

irizzant commented Oct 1, 2024

What happened?

I'm using provider-http version 1.0.3.
I have the following Request

apiVersion: http.crossplane.io/v1alpha2
kind: Request
metadata:
  name: port-team
spec:
  forProvider:
    waitTimeout: "5m"
    headers:
      Content-Type:
        - application/json
      Authorization:
        - Bearer {{${id}-port-token:crossplane-system:token}}
    payload:
      baseUrl: https://api.getport.io/v1/teams
      body: |
        {
          "name": "cost-center-${id}",
          "users": [user.email for user in parameters.users],
          "description": "Cost Center ${id} team"
        }
    mappings:
      - method: POST
        body: |
          {
            "name": .payload.body.name,
            "users": .payload.body.users,
            "description": .payload.body.description
          }
        url: .payload.baseUrl
        headers:
          Content-Type:
            - application/json
          Authorization:
            - Bearer {{${id}-port-token:crossplane-system:token}}
      - method: GET
        url: (.payload.baseUrl + "/" + .response.body.team.name)
        headers:
          Content-Type:
            - application/json
          Authorization:
            - Bearer {{${id}-port-token:crossplane-system:token}}
      - method: PUT
        body: |
          {
            "name": .payload.body.name,
            "users": .payload.body.users,
            "description": .payload.body.description
          }
        url: (.payload.baseUrl + "/" + .response.body.team.name)
        headers:
          Content-Type:
            - application/json
          Authorization:
            - Bearer {{${id}-port-token:crossplane-system:token}}
      - method: DELETE
        url: (.payload.baseUrl + "/" + .response.body.team.name)
        headers:
          Content-Type:
            - application/json
          Authorization:
            - Bearer {{${id}-port-token:crossplane-system:token}}

When I delete the MR, the Team is actually deleted successfully once but then the DELETE is retried and it fails with 404, see events order:

Events:
  Type     Reason                        Age                    From                                Message
  ----     ------                        ----                   ----                                -------
  Warning  CannotCreateExternalResource  3m40s (x6 over 3m40s)  managed/request.http.crossplane.io  something went wrong: failed to get secret 123-port-token:crossplane-system: Secret "123-port-token" not found
  Normal   CreatedExternalResource       3m39s                  managed/request.http.crossplane.io  Successfully requested creation of external resource
  Normal   UpdatedExternalResource       97s (x3 over 3m38s)    managed/request.http.crossplane.io  Successfully requested update of external resource
  Normal   DeletedExternalResource       94s                    managed/request.http.crossplane.io  Successfully requested deletion of external resource
  Warning  CannotDeleteExternalResource  28s (x6 over 93s)      managed/request.http.crossplane.io  something went wrong: HTTP DELETE request failed with status code: 404

How can we reproduce it?

What environment did it happen in?

Crossplane version: 1.17.0

@irizzant irizzant added the bug Something isn't working label Oct 1, 2024
@arielsepton
Copy link
Member

Hi,
Thanks for the details. I tried to reproduce the issue with the latest provider version, but couldn’t replicate it. I recommend upgrading and testing again. If the issue persists, feel free to share more details, and I’ll take another look.

@irizzant
Copy link
Author

irizzant commented Oct 6, 2024

Hi @arielsepton
I've just retried with provider 1.0.5 and Crossplane 1.17.1, the issue persists.

Here you can find the manifests I've tried (replace elements within <...>):

apiVersion: http.crossplane.io/v1alpha2
kind: DisposableRequest
metadata:
  name: port-token-request
spec:
  deletionPolicy: Orphan
  forProvider:
    url: https://api.getport.io/v1/auth/access_token
    method: POST
    body: |
      {
        "clientId": "{{ port:crossplane-system:port_client_id }}",
        "clientSecret": "{{ port:crossplane-system:port_client_secret }}"
      }
    headers:
      Content-Type:
        - application/json
    shouldLoopInfinitely: true
    nextReconcile: 1h
    secretInjectionConfigs:
      - secretRef:
          name: port-token
          namespace: crossplane-system
        secretKey: token
        responsePath: .body.accessToken
        setOwnerReference: true
---
apiVersion: v1
kind: Secret
metadata:
  name: port
  namespace: crossplane-system
type: Opaque
stringData:
  port_client_id: <PORT_CLIENT_ID>
  port_client_secret: <PORT_CLIENT_SECRET>
---
apiVersion: http.crossplane.io/v1alpha2
kind: Request
metadata:
  name: port-team
spec:
  forProvider:
    waitTimeout: "5m"
    headers:
      Content-Type:
        - application/json
      Authorization:
        - Bearer {{port-token:crossplane-system:token}}
    payload:
      baseUrl: https://api.getport.io/v1/teams
      body: |
        {
          "name": "cost-center-test",
          "users": ["<EMAIL>"],
          "description": "Cost Center test team"
        }
    mappings:
      - action: CREATE
        method: POST
        body: |
          {
            "name": .payload.body.name,
            "users": .payload.body.users,
            "description": .payload.body.description
          }
        url: .payload.baseUrl
        headers:
          Content-Type:
            - application/json
          Authorization:
            - Bearer {{port-token:crossplane-system:token}}
      - action: OBSERVE
        method: GET
        url: (.payload.baseUrl + "/" + .response.body.team.name)
        headers:
          Content-Type:
            - application/json
          Authorization:
            - Bearer {{port-token:crossplane-system:token}}
      - action: UPDATE
        method: PUT
        body: |
          {
            "name": .payload.body.name,
            "users": .payload.body.users,
            "description": .payload.body.description
          }
        url: (.payload.baseUrl + "/" + .response.body.team.name)
        headers:
          Content-Type:
            - application/json
          Authorization:
            - Bearer {{port-token:crossplane-system:token}}
      - action: REMOVE
        method: DELETE
        url: (.payload.baseUrl + "/" + .response.body.team.name)
        headers:
          Content-Type:
            - application/json
          Authorization:
            - Bearer {{port-token:crossplane-system:token}}

Here are the events showing the HTTP DELETE is retried:

Events:
  Type     Reason                        Age                   From                                Message
  ----     ------                        ----                  ----                                -------
  Warning  CannotCreateExternalResource  2m54s (x4 over 3m7s)  managed/request.http.crossplane.io  something went wrong: failed to get secret test-port-token:crossplane-system: Secret "test-port-token" not found
  Normal   CreatedExternalResource       2m38s                 managed/request.http.crossplane.io  Successfully requested creation of external resource
  Normal   UpdatedExternalResource       2m5s (x2 over 2m37s)  managed/request.http.crossplane.io  Successfully requested update of external resource
  Normal   DeletedExternalResource       115s                  managed/request.http.crossplane.io  Successfully requested deletion of external resource
  Warning  CannotDeleteExternalResource  49s (x6 over 114s)    managed/request.http.crossplane.io  something went wrong: HTTP DELETE request failed with status code: 404

But the actual resource is removed so the first HTTP DELETE have succeeded.

The following is the status.conditions part to show the DELETE details:

Conditions:
    Last Transition Time:  2024-10-06T09:27:28Z
    Reason:                Deleting
    Status:                False
    Type:                  Ready
    Last Transition Time:  2024-10-06T09:13:16Z
    Message:               delete failed: something went wrong: HTTP DELETE request failed with status code: 404
    Reason:                ReconcileError
    Status:                False
    Type:                  Synced
  Failed:                  19
  Request Details:
    Headers:
      Authorization:
        Bearer {{port-token:crossplane-system:token}}
      Content - Type:
        application/json
    Method:  DELETE
    URL:     https://api.getport.io/v1/teams/
  Response:
    Body:  {"ok":false,"error":"not_found","message":"Route DELETE:/v1/teams/ not found"}
    Headers:
      Connection:
        keep-alive
      Content - Length:
        78
      Content - Security - Policy:
        default-src 'self';base-uri 'self';font-src 'self' https: data:;form-action 'self';frame-ancestors 'self';img-src 'self' data:;object-src 'none';script-src 'self';script-src-attr 'none';style-src 'self' https: 'unsafe-inline';upgrade-insecure-requests
      Content - Type:
        application/json; charset=utf-8
      Cross - Origin - Embedder - Policy:
        require-corp
      Cross - Origin - Opener - Policy:
        same-origin
      Cross - Origin - Resource - Policy:
        same-origin
      Date:
        Sun, 06 Oct 2024 09:27:28 GMT
      Origin - Agent - Cluster:
        ?1
      Referrer - Policy:
        no-referrer
      Strict - Transport - Security:
        max-age=15552000; includeSubDomains
      Vary:
        Origin
      X - Content - Type - Options:
        nosniff
      X - Dns - Prefetch - Control:
        off
      X - Download - Options:
        noopen
      X - Frame - Options:
        SAMEORIGIN
      X - Permitted - Cross - Domain - Policies:
        none
      X - Xss - Protection:
        0
    Status Code:  404

@arielsepton
Copy link
Member

arielsepton commented Oct 6, 2024

Hi, I looked into this and found the issue.
All jq expressions used in the Request resource need to return null when a field is missing. Instead, gojq is returning an empty string, which is causing the problem.

To fix this, you can update your jq expression like this:

url: (.payload.baseUrl + "/" + (.response.body.team.name|tostring))

Adding tostring will ensure that the expression returns null instead of an empty string, resolving the issue. Please let me know how it goes!

@irizzant
Copy link
Author

irizzant commented Oct 7, 2024

Thanks @arielsepton I managed to get it working like this:

apiVersion: http.crossplane.io/v1alpha2
kind: Request
metadata:
  name: port-team
spec:
  forProvider:
    waitTimeout: "5m"
    headers:
      Content-Type:
        - application/json
      Authorization:
        - Bearer {{port-token:crossplane-system:token}}
    payload:
      baseUrl: https://api.getport.io/v1/teams
      body: |
        {
          "name": "cost-center-test",
          "users": ["redacted"],
          "description": "Cost Center test team"
        }
    mappings:
      - action: CREATE
        method: POST
        body: |
          {
            "name": .payload.body.name,
            "users": .payload.body.users,
            "description": .payload.body.description
          }
        url: .payload.baseUrl
        headers:
          Content-Type:
            - application/json
          Authorization:
            - Bearer {{port-token:crossplane-system:token}}
      - action: OBSERVE
        method: GET
        url: (.payload.baseUrl + "/" + (.response.body.team.name|tostring))
        headers:
          Content-Type:
            - application/json
          Authorization:
            - Bearer {{port-token:crossplane-system:token}}
      - action: UPDATE
        method: PUT
        body: |
          {
            "name": .payload.body.name,
            "users": .payload.body.users,
            "description": .payload.body.description
          }
        url: (.payload.baseUrl + "/" + (.response.body.team.name|tostring))
        headers:
          Content-Type:
            - application/json
          Authorization:
            - Bearer {{port-token:crossplane-system:token}}
      - action: REMOVE
        method: DELETE
        url: (.payload.baseUrl + "/" + (.response.body.team.name|tostring))
        headers:
          Content-Type:
            - application/json
          Authorization:
            - Bearer {{port-token:crossplane-system:token}}

But I'm facing the same problem now with PUT requests, which are repeated every minute:

Events:
  Type    Reason                   Age               From                                Message
  ----    ------                   ----              ----                                -------
  Normal  CreatedExternalResource  63s               managed/request.http.crossplane.io  Successfully requested creation of external resource
  Normal  UpdatedExternalResource  1s (x3 over 63s)  managed/request.http.crossplane.io  Successfully requested update of external resource

I had a look here, and I tried to configure the jq expression like this but PUT requests are still repeated every minute:

apiVersion: http.crossplane.io/v1alpha2
kind: Request
metadata:
  name: port-team
spec:
  forProvider:
    waitTimeout: "5m"
    headers:
      Content-Type:
        - application/json
      Authorization:
        - Bearer {{port-token:crossplane-system:token}}
    payload:
      baseUrl: https://api.getport.io/v1/teams
      body: |
        {
          "name": "cost-center-test",
          "users": ["redacted"],
          "description": "Cost Center test team"
        }
    expectedResponseCheck: # <----- see here
      type: CUSTOM
      logic: |
        if .response.body.team.name == .payload.body.name
         and .response.body.team.users == .payload.body.users
         and .response.body.team.description == .payload.body.description
         then true 
         else false 
         end
    mappings:
      - action: CREATE
        method: POST
        body: |
          {
            "name": .payload.body.name,
            "users": .payload.body.users,
            "description": .payload.body.description
          }
        url: .payload.baseUrl
        headers:
          Content-Type:
            - application/json
          Authorization:
            - Bearer {{port-token:crossplane-system:token}}
      - action: OBSERVE
        method: GET
        url: (.payload.baseUrl + "/" + (.response.body.team.name|tostring))
        headers:
          Content-Type:
            - application/json
          Authorization:
            - Bearer {{port-token:crossplane-system:token}}
      - action: UPDATE
        method: PUT
        body: |
          {
            "name": .payload.body.name,
            "users": .payload.body.users,
            "description": .payload.body.description
          }
        url: (.payload.baseUrl + "/" + (.response.body.team.name|tostring))
        headers:
          Content-Type:
            - application/json
          Authorization:
            - Bearer {{port-token:crossplane-system:token}}
      - action: REMOVE
        method: DELETE
        url: (.payload.baseUrl + "/" + (.response.body.team.name|tostring))
        headers:
          Content-Type:
            - application/json
          Authorization:
            - Bearer {{port-token:crossplane-system:token}}

Also if I may give you a suggestion, it would be much more helpful having the status field reporting not just the response of the last request.

@arielsepton
Copy link
Member

arielsepton commented Oct 7, 2024

The PUT request is being retried because your expectedResponseCheck expression is returning false—the team.users field doesn’t exist in the GET response.

To fix this, you can update the jq expression like this:

expectedResponseCheck: 
  type: CUSTOM
  logic: |
    if .response.body.team.name == .payload.body.name
    and .response.body.team.description == .payload.body.description
    then true 
    else false 
    end

As for your suggestion about adding status, could you explain a bit more about how you think it would help? Since the status only holds the latest response, I’m curious about what extra benefit you're thinking it might provide.

@irizzant
Copy link
Author

irizzant commented Oct 7, 2024

Thanks @arielsepton for the updates.

But what happens if I change a user in the users field? The logic you suggested would consider the resource updated because it doesnt't use that field right?

In that case I think that's a flaw in the API.

About the status field, as you said it reports the status of the latest response. I was thinking that it would be good to know the status of the previous requests as well, because if I have an error I can't see any details about it by inspecting the resource itself.

@arielsepton
Copy link
Member

arielsepton commented Oct 7, 2024

I spoke with a tech lead at Port @danielsinai, and it turns out if you add the query parameters ?fields=name&fields=description&fields=users.email, you can retrieve the users in this format:

 "users": [{"email": "<email>"}, {"email": "<email>"}] 

You can update your resource like this:

apiVersion: http.crossplane.io/v1alpha2
kind: Request
metadata:
  name: port-team
spec:
  forProvider:
    waitTimeout: "5m"
    headers:
      Content-Type:
        - application/json
      Authorization:
        - Bearer {{port-token:crossplane-system:token}}
    payload:
      baseUrl: https://api.getport.io/v1/teams
      body: |
        {
          "name": "test",
          "users": ["<email>"],
          "description": "Cost Center test team"
        }
    expectedResponseCheck: 
      type: CUSTOM
      logic: |
        if .response.body.team.name == .payload.body.name
         and (.response.body.team.users | map(.email) | sort) == (.payload.body.users | sort)
         and .response.body.team.description == .payload.body.description
         then true 
         else false 
         end
    mappings:
      - action: CREATE
        method: POST
        body: |
          {
            "name": .payload.body.name,
            "users": .payload.body.users,
            "description": .payload.body.description
          }
        url: .payload.baseUrl
      - action: OBSERVE
        method: GET
        url: (.payload.baseUrl + "/" + (.response.body.team.name|tostring) + "?fields=name&fields=description&fields=users.email")
      - action: UPDATE
        method: PUT
        body: |
          {
            "name": .payload.body.name,
            "users": .payload.body.users,
            "description": .payload.body.description
          }
        url: (.payload.baseUrl + "/" + (.response.body.team.name|tostring))
      - action: REMOVE
        method: DELETE
        url: (.payload.baseUrl + "/" + (.response.body.team.name|tostring))

This should solve the issue.

Regarding the status field, while tracking previous request statuses could be helpful for debugging, it might make the field too large and hard to manage. I think adding more detailed logs could give you the info you need without cluttering the resource, making it easier to trace past requests while keeping status focused on the current state.

@irizzant
Copy link
Author

irizzant commented Oct 7, 2024

Thanks @arielsepton , that worked.

For me having the requests logs e.g. at provider level is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants