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: Allow setting local_context_data #99

Merged
merged 2 commits into from
Apr 27, 2022
Merged

Conversation

amhn
Copy link
Contributor

@amhn amhn commented Apr 17, 2022

This PR fixes #59

local_context_data still needs to be a string in the resource due to limitations in terraform.
This fix needs smutel/go-netbox#22 merged and updated in vendor/.

local_context_data can be specified as follows or as string

resource "netbox_virtualization_vm" "virtual_machines" {
  […]
      local_context_data = jsonencode(
            {
                    foo = "bar"
                    number = 1
            }
        )
}

@auto-add-label auto-add-label bot added the bug Something isn't working label Apr 17, 2022
@auto-assign auto-assign bot requested a review from smutel April 17, 2022 20:10
@smutel
Copy link
Owner

smutel commented Apr 24, 2022

Hello. Could you please rebase your branch and use go-netbox/v3.0.1. Thanks.

@amhn amhn force-pushed the local_context_data branch from 2813aa6 to 5ba20c6 Compare April 24, 2022 20:10
@smutel
Copy link
Owner

smutel commented Apr 27, 2022

Hello,

It seems that the master in your fork of my project is not up to date.
For testing, it will be easier for me to rebase your branch from the master of this project.

Thanks.

@amhn
Copy link
Contributor Author

amhn commented Apr 27, 2022

Done. Pushed your master to my fork. Changes for this PR are in https://github.com/amhn/terraform-provider-netbox/tree/local_context_data

@smutel
Copy link
Owner

smutel commented Apr 27, 2022

I tested this code with Netbox 3.0.11 and I have the error below:

netbox_virtualization_vm.vm_test: Modifying... [id=23]
╷
│ Error: response status code does not match any response statuses defined for this endpoint in the swagger spec (status 400): {}
│ 
│   with netbox_virtualization_vm.vm_test,
│   on main.tf line 271, in resource "netbox_virtualization_vm" "vm_test":
│  271: resource "netbox_virtualization_vm" "vm_test" {

@amhn
Copy link
Contributor Author

amhn commented Apr 27, 2022

I just pushed a new commit with a fix if local_context_data is empty or not defined. I did not test that case.

I tried different things and could not reproduce a status code of 400. Also using netbox-3.0.11

The following all worked:

  • no local_context_data
  • local_context_data = ""
  • local_context_data = jsonencode({})
  • local_context_data = jsonencode({foo = "bar"})

Which branch did you test with? My master is identical to yours, changes are in the local_context_data branch.
Can you give me a .tf file that fails?
Can you record the traffic between netbox and terraform with e.g. tcpdump?

I suspect you were on the wrong branch. Because that is the behaviour before my changes.

(That the error message from the 400 is not printed, is another issue worth investigating. I suspect this is due to swagger.json not mentioning 400 as a possible response status)

@smutel
Copy link
Owner

smutel commented Apr 27, 2022

My bad, I tested with your master branch. Everything looks good. Thanks.

@smutel smutel merged commit 4f1844d into smutel:master Apr 27, 2022
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 this pull request may close these issues.

local_context_data on virtual machine is asking for a string but needs te be JSON
2 participants