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

Support 'PUT' requests that send parameters via the query string #2040

Open
jgangemi opened this issue Oct 15, 2024 · 10 comments · May be fixed by #2041
Open

Support 'PUT' requests that send parameters via the query string #2040

jgangemi opened this issue Oct 15, 2024 · 10 comments · May be fixed by #2041

Comments

@jgangemi
Copy link
Contributor

Hot on the trails of #2039, I've discovered that the current implementation of NewRequest doesn't work properly w/ the integrations api endpoint. those methods require the parameters to be sent as part of the query string and not as a json body in the request.

see: https://docs.gitlab.com/ee/api/integrations.html#set-up-gitlab-for-slack-app as an example.

@svanharmelen
Copy link
Member

I'm looking at the link you've send but it doesn't mention anything about this having to be a PUT request with query params. Would you mind sending me info that shows this is actually the case? Maybe a link to the related GitLap API code?

@jgangemi
Copy link
Contributor Author

that might take some serious spelunking.

i can post some output from a test program i wrote in the morning to demonstrate it not working and see what else i can dig up.

@jgangemi
Copy link
Contributor Author

i can also share this at the moment which shows the reporter trying to set the attributes using a query string: https://gitlab.com/gitlab-org/gitlab/-/issues/28903

@svanharmelen
Copy link
Member

I send the other update in the middle of the night (CEST) as I was woken by a page, but I thought to remember something about the API actually accepting both query strings and payload body and I found the docs mentioning this: https://docs.gitlab.com/ee/api/rest/#request-payload

If you read closely, you also notice that they write that usually only GET requests use query strings and POST and PUT requests use payload body (quite the default with REST). So that matches exactly with what this package does and so I don't know how you came to the conclusion that this needed to be a call that specifically needs to use query params, but I think your assumption might be incorrect...

@jgangemi
Copy link
Contributor Author

jgangemi commented Oct 16, 2024

this does not work, note the value of inherited in the response.

curl -X PUT --header "PRIVATE-TOKEN: <token>" https://gitlab.com/api/v4/projects/<project-id>/integrations/gitlab-slack-application --data-raw '{"use_inherited_settings": false"}' | jq
{
  "id": 128662996,
  "title": "GitLab for Slack app",
  "slug": "gitlab-slack-application",
  "created_at": "2024-10-15T01:57:19.376Z",
  "updated_at": "2024-10-16T11:30:53.999Z",
  "active": true,
  "commit_events": false,
  "push_events": false,
  "issues_events": false,
  "incident_events": false,
  "alert_events": false,
  "confidential_issues_events": false,
  "merge_requests_events": false,
  "tag_push_events": false,
  "deployment_events": false,
  "note_events": false,
  "confidential_note_events": false,
  "pipeline_events": false,
  "wiki_page_events": false,
  "job_events": false,
  "comment_on_event_enabled": true,
  "inherited": true,
  "vulnerability_events": false,
  "properties": {
    "channel": null,
    "notify_only_broken_pipelines": true,
    "branches_to_be_notified": "default",
    "labels_to_be_notified": null,
    "labels_to_be_notified_behavior": "match_any",
    "push_channel": null,
    "issue_channel": null,
    "confidential_issue_channel": null,
    "merge_request_channel": null,
    "note_channel": null,
    "confidential_note_channel": null,
    "tag_push_channel": null,
    "pipeline_channel": null,
    "wiki_page_channel": null,
    "deployment_channel": null,
    "incident_channel": null,
    "vulnerability_channel": null,
    "alert_channel": null
  }
}

this does:

curl -X PUT --header "PRIVATE-TOKEN: <token>" https://gitlab.com/api/v4/projects/<project_id>/integrations/gitlab-slack-application\?use_inherited_settings\=false | jq
{
  "id": 128662996,
  "title": "GitLab for Slack app",
  "slug": "gitlab-slack-application",
  "created_at": "2024-10-15T01:57:19.376Z",
  "updated_at": "2024-10-16T11:34:08.852Z",
  "active": true,
  "commit_events": false,
  "push_events": false,
  "issues_events": false,
  "incident_events": false,
  "alert_events": false,
  "confidential_issues_events": false,
  "merge_requests_events": false,
  "tag_push_events": false,
  "deployment_events": false,
  "note_events": false,
  "confidential_note_events": false,
  "pipeline_events": false,
  "wiki_page_events": false,
  "job_events": false,
  "comment_on_event_enabled": true,
  "inherited": false,
  "vulnerability_events": false,
  "properties": {
    "channel": null,
    "notify_only_broken_pipelines": true,
    "branches_to_be_notified": "default",
    "labels_to_be_notified": null,
    "labels_to_be_notified_behavior": "match_any",
    "push_channel": null,
    "issue_channel": null,
    "confidential_issue_channel": null,
    "merge_request_channel": null,
    "note_channel": null,
    "confidential_note_channel": null,
    "tag_push_channel": null,
    "pipeline_channel": null,
    "wiki_page_channel": null,
    "deployment_channel": null,
    "incident_channel": null,
    "vulnerability_channel": null,
    "alert_channel": null
  }
}

@svanharmelen
Copy link
Member

The first command seems to have a bug, the JSON you post contains a " after the word false Can you verify/confirm this?

@jgangemi
Copy link
Contributor Author

you're right it does, here's an update. still doesn't work:

curl -s -X PUT --header "PRIVATE-TOKEN: <token>" https://gitlab.com/api/v4/projects/<project-id>/integrations/gitlab-slack-application --data-raw '{"use_inherited_settings":false}' 
{
  "id": 128662996,
  "title": "GitLab for Slack app",
  "slug": "gitlab-slack-application",
  "created_at": "2024-10-15T01:57:19.376Z",
  "updated_at": "2024-10-16T13:31:33.351Z",
  "active": true,
  "commit_events": false,
  "push_events": false,
  "issues_events": false,
  "incident_events": false,
  "alert_events": false,
  "confidential_issues_events": false,
  "merge_requests_events": false,
  "tag_push_events": false,
  "deployment_events": false,
  "note_events": false,
  "confidential_note_events": false,
  "pipeline_events": false,
  "wiki_page_events": false,
  "job_events": false,
  "comment_on_event_enabled": true,
  "inherited": true,
  "vulnerability_events": false,
  "properties": {
    "channel": null,
    "notify_only_broken_pipelines": true,
    "branches_to_be_notified": "default",
    "labels_to_be_notified": null,
    "labels_to_be_notified_behavior": "match_any",
    "push_channel": null,
    "issue_channel": null,
    "confidential_issue_channel": null,
    "merge_request_channel": null,
    "note_channel": null,
    "confidential_note_channel": null,
    "tag_push_channel": null,
    "pipeline_channel": null,
    "wiki_page_channel": null,
    "deployment_channel": null,
    "incident_channel": null,
    "vulnerability_channel": null,
    "alert_channel": null
  }
}

also, i was reading this: https://docs.gitlab.com/ee/api/rest/#request-payload and it states:

while PUT or POST requests ** usually ** send the payload body:

so it seems using a payload may not always be the case, as seems to be evidenced here.

i had started working on changes for #2039 and see you posted #2042 already, so i'll try that out as is and report back but i still think this is going to be an issue even w/ those changes.

@svanharmelen
Copy link
Member

svanharmelen commented Oct 16, 2024

I would almost argue that this is a bug in the GitLab API and its probably wise opening a case there to get to the bottom of this, as this is first time in ~8 years someone reported this as a problem.

@jgangemi
Copy link
Contributor Author

i can do that however i'm skeptical it will be fixed any time soon and i'd really like to create a terraform resource that can manage the slack application since we have a lot of repos and configuring it via click-ops isn't really practical.

@svanharmelen
Copy link
Member

I understand, but I'm not really looking forward (and so not really open) to introduce "hacks" in order to workaround bugs in the GitLab API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants