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

HTTP Error does not prevent restapi_object state from being updated #152

Closed
jollyroger opened this issue Oct 16, 2021 · 9 comments · Fixed by #265
Closed

HTTP Error does not prevent restapi_object state from being updated #152

jollyroger opened this issue Oct 16, 2021 · 9 comments · Fixed by #265
Labels
bug Something isn't working

Comments

@jollyroger
Copy link

jollyroger commented Oct 16, 2021

Hi! First of all, thanks for your great work! I'm facing an issue when HTTP API call fails due to an error(HTTP 406). Usually this means the change was not applied and I'd be happy to update object properties and run terraform apply again to retry. But during second run terraform shows that no changes will be applied because restapi_object resource state was already updated during previous run.

Is there any way to prevent state update in case of HTTP Error?

Version info:

  • Terraform: 0.12.10
  • terraform-provider-restapi: 1.16.0
@laudibert
Copy link

laudibert commented Dec 1, 2021

This is a big issue as 400+ errors should indicate that no changes were made, hence the state should not be updated.

There might be cases where this is not the case, but that would be the exception rather than the norm.

@DRuggeri
Copy link
Member

DRuggeri commented Apr 7, 2022

Hey there, @jollyroger - I'm embarrassed by the delay responding. Somehow I opened this report marking it as 'read' in GitHub... but I never actually READ it!

I will have to work on a reproduction recipe for this to triage - totally agreed that 4xx errors would indicate the request was never accepted by the API so a state change wouldn't make sense.

If you have a handy way to make this easily reproducible (maybe with fakeserver), let me know and it can shortcut some of the process :-)

@DRuggeri DRuggeri added the bug Something isn't working label Apr 7, 2022
@othman-essabir
Copy link

othman-essabir commented Apr 8, 2022

Hi there,
wouldn't it be cool to provide an optional parameter with a list of acceptable http codes

@jollyroger
Copy link
Author

jollyroger commented May 21, 2022

Hi @DRuggeri , sorry to keep you waiting, here are two files for you to test this case.

terraform {
  required_providers {
    restapi = {
      source  = "Mastercard/restapi"
    }
  }
}

provider "restapi" {
  uri                  = "http://localhost:8000"
  debug                = false
  write_returns_object = true
  id_attribute         = "id"
}

resource "random_uuid" "uuid" {count = 2}

resource "restapi_object" "redis_db" {
  path        = "/objects"
  update_path = "/objects/{id}"
  data        = jsonencode({"property" = random_uuid.uuid[0].result})
}

This is a terraform file I'm using. By changing uuid[0] to uuid[1] and vice versa you'll be able to easily provide changes for terraform to apply. If you then try to rename property to something else, you'll get HTTP 406 error.

Here's a server code written in Python. You'd need to install few Python packages first by running apt install python3-bottle python3-bottle-sqlite. Afterwards just run the server with python3 server.py and visit http://localhost:8000/db/init to create database. Once done you can get back to Terraform code. I have left the code in the comments where you can change Error Code itself on the server

#!/usr/bin/env python3

import io
import bottle
import json

from bottle_sqlite import SQLitePlugin

bottle.install(SQLitePlugin(dbfile="db.sqlite"))
app = application = bottle.default_app()

@app.route('/db/init')
def populate(db):
    db.execute("CREATE TABLE IF NOT EXISTS objects(id INT PRIMARY KEY, data BLOB)")
    bottle.response.status = 200
    bottle.response.headers['Content-Type'] = 'application/json'
    return json.dumps({'status':'ok', 'description':'Database initialized'})


@app.route('/objects', method=['GET'])
def list_items(db):
    res = db.execute("SELECT ROWID, DATA from objects")
    bottle.response.status = 200
    bottle.response.headers['Content-Type'] = 'application/json'
    columns = ["id", "data"]
    return json.dumps([dict(zip(columns, tuple(row))) for row in res])


@app.route('/objects', method=['POST'])
def create_item(db):
    wrapper = io.TextIOWrapper(bottle.request.body, encoding='utf-8')
    new_data = wrapper.read()
    res = db.execute("INSERT INTO objects(data) VALUES(?)", (new_data,))
    return get_item(db, res.lastrowid)

@app.route('/objects/<object_id:int>', method=['GET'])
def get_item(db, object_id):
    res = db.execute("SELECT ROWID, DATA from objects where ROWID = ?", (object_id,))
    row = tuple(res.fetchone())
    columns = ["id", "data"]
    bottle.response.status = 200 if len(row) > 0 else 404
    bottle.response.headers['Content-Type'] = 'application/json'
    return json.dumps(dict(zip(columns, tuple(row))))


@app.route('/objects/<object_id:int>', method=['PUT'])
def update_item(db, object_id):
    try:
        new_data = bottle.request.json
        if new_data is None or "property" not in new_data:
            raise ValueError
    except ValueError:
        bottle.response.status = 406
        # ^^^^^ Update HTTP Error Code here ^^^^^
        return json.dumps({'status':'ko', 'description': 'Malformed JSON Object'})
    wrapper = io.TextIOWrapper(bottle.request.body, encoding='utf-8')
    new_data = wrapper.read()
    res = db.execute("UPDATE objects SET data = ? WHERE ROWID = ?", (new_data, object_id))
    return get_item(db, object_id)


if __name__ == '__main__':
        bottle.run(app, host = '127.0.0.1', port = 8000)

@jollyroger
Copy link
Author

Sorry, forgot to add steps to reproduce:

  • Install deps and run server (python3 server.py)
  • run terraform apply for existing terraform code
  • (optional) change random_uuid.uuid[0].result to random_uuid.uuid[1].result to ensure updating objects works
  • update terraform code by replacing this line:
-data        = jsonencode({"property" = random_uuid.uuid[0].result})
+data        = jsonencode({"new_property" = random_uuid.uuid[0].result})
  • run terraform apply, observe HTTP 406 error response
  • re-apply once again, observe terraform reporting everything is up to date (which is wrong because object wasn't actually updated on previous step)

Expected result:

  • detect object not in desired state
  • generate plan to update object

ugur-zongur pushed a commit to ugur-zongur/terraform-provider-restapi that referenced this issue Mar 28, 2024
@ugur-zongur
Copy link

Applying the workaround from hashicorp/terraform-plugin-sdk#476 in the PR above. I also verified the change with terraform v1.6.3 and v1.7.5 in a similar way to what @jollyroger suggested.

@ugur-zongur
Copy link

This issue has been around for quite some time and it's occasionally causing problems to us. A review for the fix would be greatly appreciated. I'm not sure who is the right person to ping but judging by the commit activity, maybe @DRuggeri ?

@DRuggeri
Copy link
Member

DRuggeri commented Aug 5, 2024

This is awesome, @ugur-zongur - thank you for the PR! Merging now!

@DRuggeri
Copy link
Member

DRuggeri commented Aug 5, 2024

As noted in the PR feedback, I plan to release this soon along with the fix for #231 - massive thank you for the PR and for validating the fix in your local environment, @ugur-zongur !

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

Successfully merging a pull request may close this issue.

5 participants