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

Added custom fields resource (VM's only) #158

Merged
merged 6 commits into from
May 6, 2022

Conversation

chapsuk
Copy link
Contributor

@chapsuk chapsuk commented Apr 21, 2022

Introducing custom_fields only for virtual machines instead of all resource

@chapsuk chapsuk mentioned this pull request Apr 21, 2022
@fbreckle
Copy link
Collaborator

Ok I tried this and without custom fields this is now not annoying anymore :D
I noticed that, generally, if a custom field is introduced for a netbox type, that field keeps defaulting to an empty string if I do not set it in the resource

  # netbox_virtual_machine.testvm will be updated in-place
  ~ resource "netbox_virtual_machine" "testvm" {
      ~ custom_fields = {
          - "testfield" = "" -> null
        }
        id            = "1"
        name          = "my-test-vm"
        tags          = [
            "bar",
            "foo",
        ]
        # (10 unchanged attributes hidden)
    }

This means, that if a custom field is defined for a netbox type, to prevent this from happening, all terraform-managed resources of that type have to have this field set.

Since this happens only if a custom field is defined, I think this is okay.

Continuing this thought, maybe we can implement a provider-wide setting later that allows a terraform module to just ignore all custom fields or a subset. But this is not in scope for this MR.

Will review the rest later, I just wanted to check on the drift-detection spam.

@arjenvri
Copy link
Contributor

arjenvri commented May 5, 2022

Nice work! Would it possible to include custom fields support for sites as well ?

chapsuk#1

@fbreckle
Copy link
Collaborator

fbreckle commented May 6, 2022

He made a MR with all the custom fields already at #143.
There were some issues, so we decided to start with VMs first and see how it goes. If all goes well, I definitely want to include custom fields for the other resources as well.

@fbreckle fbreckle merged commit cd7f541 into e-breuninger:master May 6, 2022
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 this pull request may close these issues.

3 participants