Skip to content
This repository has been archived by the owner on Dec 26, 2023. It is now read-only.

API endpoints mishandle trailing slash #496

Closed
Spritekin opened this issue Jul 2, 2023 · 6 comments · Fixed by #516 or #517
Closed

API endpoints mishandle trailing slash #496

Spritekin opened this issue Jul 2, 2023 · 6 comments · Fixed by #516 or #517

Comments

@Spritekin
Copy link
Contributor

Spritekin commented Jul 2, 2023

Hi,
I have been testing the new autorun function using Terrasnek and it is working, however there is a new bug I noticed during the variable registration.
My python program creates a Git-backed workspace, then registers four variables then starts a run. Note I know this code works because I have been using it in TFC for a while.
The output from the python program is as follows:

Searching if workspace oss-user-otf exists...
User workspace oss-user-otf not found, creating...
workspace ws-9fS9KGaMoPwQB4ER created
Registering acces keys...
{'data': []}  # NOTICE IT RETURNS A SUCCESS BUT EMPTY DATA
{'data': []}  # API SAYS IT SHOULD RETURN VARIABLE SPECIFIC INFO
Registering tokens...
{'data': []}  # SAME IN THIS PAIR
{'data': []}
User workspace created, starting the first run...
{'data': {'id': 'run-U4x9NCinYk6uI3Sv', ... etc etc etc}
First run created run-U4x9NCinYk6uI3Sv

So you can see the variables are not created then the run fails.

Now, from the OTF logs, there is no mention about the variable creation. There is the workspace creation log and the run log, but there is no variable creation log.

2023/07/02 12:16:19 INFO created workspace id=ws-9fS9KGaMoPwQB4ER name=oss-user-otf organization=Spritekin subject=Spritekin
# THE LOGS FOR VARIABLE CREATION SHOULD BE HERE!!!!
2023/07/02 12:16:31 INFO enqueued plan id=run-U4x9NCinYk6uI3Sv subject=scheduler
2023/07/02 12:16:31 INFO started plan id=run-U4x9NCinYk6uI3Sv subject=local-agent
2023/07/02 12:16:31 INFO executing phase component=agent run=run-U4x9NCinYk6uI3Sv phase=plan
2023/07/02 12:16:31 ERROR retrieving current state version error="resource not found" workspace_id=ws-9fS9KGaMoPwQB4ER subject=local-agent
2023/07/02 12:16:50 ERROR executing phase component=agent run=run-U4x9NCinYk6uI3Sv phase=plan error="1 error occurred:\n\t* exit status 1: Error: No value for required variable on configure.tf line 8: 8: variable \"otf_token\" {} The root module input variable \"otf_token\" is not set, and has no default value. Use a -var or -var-file command line argument to provide a value for this variable. Error: No value for required variable on configure.tf line 9: 9: variable \"tfc_token\" {} The root module input variable \"tfc_token\" is not set, and has no default value. Use a -var or -var-file command line argument to provide a value for this variable.\n\n"
2023/07/02 12:16:50 INFO finishing phase component=agent run=run-U4x9NCinYk6uI3Sv phase=plan
2023/07/02 12:16:50 INFO finished plan id=run-U4x9NCinYk6uI3Sv report=+0/~0/−0 subject=local-agent

Now I know the API is handling the endpoint at:

r.HandleFunc("/workspaces/{workspace_id}/vars", a.create).Methods("POST")

And the function is implemented a few lines below:
https://github.com/leg100/otf/blob/5cb3cb1e34f28a6b2ab5d7faa940e76656585831/internal/api/variable.go#L27C6-L27C7

I have followed the function down to:

func (pdb *pgdb) create(ctx context.Context, v *Variable) error {

func (q *DBQuerier) InsertVariable(ctx context.Context, params InsertVariableParams) (pgconn.CommandTag, error) {

All looks good, but the result is still empty and with no error message at all.

This is the python code and payloads I use to create two of the variables:

# Registers the access and secret keys in the environment
def register_access_keys(tfc, workspace_id, aws_access_key_id, aws_secret_access_key):
    print("Registering acces keys...")
    master_aws_access_key_id = {
      "data": {
        "type":"vars",
        "attributes": {
          "key": "AWS_ACCESS_KEY_ID",
          "value": aws_access_key_id,
          "description": "Bot master access key",
          "category": "env",
          "hcl": False,
          "sensitive": True
        }
      }
    }
    res = tfc.workspace_vars.create(workspace_id, master_aws_access_key_id)
    print(res)

    master_aws_secret_access_key = {
      "data": {
        "type":"vars",
        "attributes": {
          "key": "AWS_SECRET_ACCESS_KEY",
          "value": aws_secret_access_key,
          "description": "Bot master secret access key",
          "category": "env",
          "hcl": False,
          "sensitive": True
        }
      }
    }
    res = tfc.workspace_vars.create(workspace_id, master_aws_secret_access_key)
    print(res)

Again I know this code works fine in TFC.
The interface I use for in the "tfc" object above is a Terrasnek object. I can provide a more complete code if interested.

@leg100
Copy link
Owner

leg100 commented Jul 3, 2023

I'll need more info:

  • Turn on http logging in OTF: otfd --log-http-requests
  • Turn on debug logging in terrasnek (I noticed it referring to debug in its code)

And if nothing suspicious turns up, please create a variable via curl:

https://developer.hashicorp.com/terraform/cloud-docs/api-docs/variables#sample-request

@Spritekin
Copy link
Contributor Author

Spritekin commented Jul 3, 2023

Hi @leg100
I turned on logging for the requests library. Here is the output. I have annotated it with comments starting with #

# This is the starting connection and request of the well-known points
DEBUG:urllib3.connectionpool:Starting new HTTPS connection (1): otf.oss.ossy.dev:443
DEBUG:urllib3.connectionpool:https://otf.oss.ossy.dev:443 "GET /.well-known/terraform.json HTTP/1.1" 200 283
DEBUG:urllib3.connectionpool:Starting new HTTPS connection (1): pypi.org:443
DEBUG:urllib3.connectionpool:https://pypi.org:443 "GET /pypi/terrasnek/json HTTP/1.1" 200 11055

# Here I get the list of workspaces to find out if a workspace with the requested name exists (notice method GET)
Searching if workspace oss-user-otf exists...
DEBUG:urllib3.connectionpool:Starting new HTTPS connection (1): otf.oss.ossy.dev:443
DEBUG:urllib3.connectionpool:https://otf.oss.ossy.dev:443 "GET /api/v2/organizations/Spritekin/workspaces HTTP/1.1" 200 1320

# The workspace does not exist so I request a workspace creation (notice method POST)
User workspace oss-user-otf not found, creating...
DEBUG:urllib3.connectionpool:Starting new HTTPS connection (1): otf.oss.ossy.dev:443
DEBUG:urllib3.connectionpool:https://otf.oss.ossy.dev:443 "POST /api/v2/organizations/Spritekin/workspaces HTTP/1.1" 201 1218
workspace ws-tXxC4AFtf5e3V9Ko created

# The workspace is created, so I start creating the 4 variables, now notice the POST is 301'd into a GET !!!!!!!
Registering acces keys...
DEBUG:urllib3.connectionpool:Starting new HTTPS connection (1): otf.oss.ossy.dev:443
DEBUG:urllib3.connectionpool:https://otf.oss.ossy.dev:443 "POST /api/v2/workspaces/ws-tXxC4AFtf5e3V9Ko/vars/ HTTP/1.1" 301 0
DEBUG:urllib3.connectionpool:https://otf.oss.ossy.dev:443 "GET /api/v2/workspaces/ws-tXxC4AFtf5e3V9Ko/vars HTTP/1.1" 200 11
{'data': []}
DEBUG:urllib3.connectionpool:Starting new HTTPS connection (1): otf.oss.ossy.dev:443
DEBUG:urllib3.connectionpool:https://otf.oss.ossy.dev:443 "POST /api/v2/workspaces/ws-tXxC4AFtf5e3V9Ko/vars/ HTTP/1.1" 301 0
DEBUG:urllib3.connectionpool:https://otf.oss.ossy.dev:443 "GET /api/v2/workspaces/ws-tXxC4AFtf5e3V9Ko/vars HTTP/1.1" 200 11
{'data': []}
Registering tokens...
DEBUG:urllib3.connectionpool:Starting new HTTPS connection (1): otf.oss.ossy.dev:443
DEBUG:urllib3.connectionpool:https://otf.oss.ossy.dev:443 "POST /api/v2/workspaces/ws-tXxC4AFtf5e3V9Ko/vars/ HTTP/1.1" 301 0
DEBUG:urllib3.connectionpool:https://otf.oss.ossy.dev:443 "GET /api/v2/workspaces/ws-tXxC4AFtf5e3V9Ko/vars HTTP/1.1" 200 11
{'data': []}
DEBUG:urllib3.connectionpool:Starting new HTTPS connection (1): otf.oss.ossy.dev:443
DEBUG:urllib3.connectionpool:https://otf.oss.ossy.dev:443 "POST /api/v2/workspaces/ws-tXxC4AFtf5e3V9Ko/vars/ HTTP/1.1" 301 0
DEBUG:urllib3.connectionpool:https://otf.oss.ossy.dev:443 "GET /api/v2/workspaces/ws-tXxC4AFtf5e3V9Ko/vars HTTP/1.1" 200 11
{'data': []}

# And now the run is started, this is a normal POST.
User workspace created, starting the first run...
DEBUG:urllib3.connectionpool:Starting new HTTPS connection (1): otf.oss.ossy.dev:443
DEBUG:urllib3.connectionpool:https://otf.oss.ossy.dev:443 "POST /api/v2/runs HTTP/1.1" 201 982
{'data': {'id': 'run-Cibf6tyqivHNBjHa', ... }}
First run created run-Cibf6tyqivHNBjHa

So why is the POST for that particular method turned into a 301? Maybe is the trailing "/" after the "vars"? Notice how the post command ends as "POST /api/v2/workspaces/ws-tXxC4AFtf5e3V9Ko/vars/"

Many browsers are very special with extra forward slashes.
I will enable the logs in otfd and see what happens there.

@Spritekin
Copy link
Contributor Author

I was looking at the Terrasnek code, yes it is sending a trailing slash in that endpoint.
https://github.com/dahlke/terrasnek/blob/a3bb4d126f21718d73b7563343131c590c736c07/terrasnek/workspace_vars.py#L26C5-L37C42

It is not ideal but the OTF API should be able to handle it.

@Spritekin
Copy link
Contributor Author

Spritekin commented Jul 3, 2023

Yes I tested with curl, this is the result. I created a simple payload:

{
  "data": {
    "type":"vars",
    "attributes": {
      "key":"some_key",
      "value":"some_value",
      "description":"some description",
      "category":"terraform",
      "hcl":false,
      "sensitive":false
    }
  }
}

The I run a curl, note the endpoint without the trailing slash.

% curl --header "Authorization: Bearer eyJhbG___MYTOKEN___7lglc" \
  --header "Content-Type: application/vnd.api+json" \
  --request POST \
  --data @payload.json \
  https://otf.oss.ossy.dev/api/v2/workspaces/ws-tXxC4AFtf5e3V9Ko/vars
{"data":{"id":"var-65ojIdmDE1NbRoJ2","type":"vars","attributes":{"category":"terraform","description":"some description","hcl":false,"key":"some_key","sensitive":false,"value":"some_value"},"relationships":{"configurable":{"data":{"id":"ws-tXxC4AFtf5e3V9Ko","type":"workspaces"}}}}}

Notice it responds just fine.

Now I the same experiment but I added a trailing slash (I deleted the variable created in the step before).

% curl --header "Authorization: Bearer eyJhbG___MYTOKEN___F7lglc" \
  --header "Content-Type: application/vnd.api+json" \
  --request POST \
  --data @payload.json \
  https://otf.oss.ossy.dev/api/v2/workspaces/ws-tXxC4AFtf5e3V9Ko/vars/
# LOTS OF CURL VERBOSE OUTPUT
< HTTP/2 301 
< date: Mon, 03 Jul 2023 11:55:50 GMT
< content-length: 0
< location: /api/v2/workspaces/ws-tXxC4AFtf5e3V9Ko/vars
< tfp-api-version: 2.5
< strict-transport-security: max-age=15724800; includeSubDomains

Notice in this case the 301 and the new location is without the forward slash.

This code works fine in TFC so there should be a way to handle the ending slash without causing a 301. Or maybe return a 307 which allows for a POST follow?

@Spritekin
Copy link
Contributor Author

I changed my python code to use a custom function for that endpoint that does not include the trailing slash and it works fine. This sseems like a minor issue but makes OTF non-Terrasnek compatible.

@leg100 leg100 changed the title Create Workspace Variable failing silently API endpoints mishandle trailing slash Jul 5, 2023
@leg100
Copy link
Owner

leg100 commented Jul 5, 2023

I'll make a change to all endpoints, to accept a trailing slash, and to disable redirects.

I've renamed the issue accordingly.

leg100 added a commit that referenced this issue Jul 8, 2023
leg100 pushed a commit that referenced this issue Jul 12, 2023
🤖 I have created a release *beep* *boop*
---


## [0.0.53](v0.0.52...v0.0.53)
(2023-07-12)


### Bug Fixes

* delete existing unreferenced webhooks too
([6b61b48](6b61b48))
* delete webhooks when org or vcs provider is deleted
([#518](#518))
([0d36ea5](0d36ea5))
* **docs:** version using tag not branch name
([8613fe8](8613fe8))
* only set not null after populating column
([1da3936](1da3936))
* remove trailing slash from requests
([#516](#516))
([c1ee39e](c1ee39e)),
closes [#496](#496)
* **ui:** add cache-control header to static files
([061261f](061261f))


### Miscellaneous

* add hashes to all static urls
([3650926](3650926))
* test create connected workspace via api
([9bf4bae](9bf4bae))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants