terraform-coop / terraform-provider-foreman

Terraform provider for Foreman
https://registry.terraform.io/providers/terraform-coop/foreman
Mozilla Public License 2.0
34 stars 32 forks source link

Smart Class parameters #6

Closed galindro closed 2 years ago

galindro commented 4 years ago

Is that hard to support Smart Class parameters?

lhw commented 4 years ago

Seems simple enough according to the API documentation in Foreman: https://theforeman.org/api/1.20/apidoc/v2/smart_variables.en.html I just hadn't had the use for them yet as we don't use puppet, so I don't really have a good test case.

galindro commented 4 years ago

I can help you with that. My company's largely uses puppet. Just let me know

lhw commented 4 years ago

If you want to implement it I'll gladly accept a PR otherwise I will have to see when I have time for this. I still need to do the Foreman >1.20, well 2.0 now, update too.

galindro commented 4 years ago

I'm not a golang dev... I can wait, np. In the meanwhile I'm direct using the foreman API to update those smart class parameters

lhw commented 3 years ago

I played around with adding smart parameters. Due to the slightly weird mechanisms of smart parameters I wanted to ask what exactly you would expect from smart parameters in terraform. As far as I can tell you can only do read and update via the API. So a resource is basically not possible as neither create nor delete would work.

galindro commented 3 years ago

Hi @lhw. Well, I don't need to create a smart parameter. I need to override the default value of it, by specifying matchers that would apply a given value to a given hostgroup, host, fqdn, etc.... Like this:

image

lhw commented 3 years ago

Thats what I thought it would be. I don't know if that is possible. Maybe I could consider the smart class parameter an override which is a resource in itself. It's definitely not an easy one

galindro commented 3 years ago

It is possible to create/delete/update those overrides easily via API. I made a python script that does it, to workaround this issue.

https://theforeman.org/api/2.1/

image

lhw commented 3 years ago

See. I didn't know that that also existed. That helps a lot. I will check on that tomorrow how quick that would be to implement. Shouldn't be hard.

lhw commented 3 years ago

So what would be more in line how you would use the overrides This?

resource "foreman_smart_class_parameter_override" "test" {
  smart_class_parameter_id = "string"

  override {
    match = "fqdn=temp.yourdomain.net"
    value = 8080
    omit  = false
  }

  override {
    match = "hostgroup=Common"
    value = 8080
    omit  = false
  }
}

or

resource "foreman_override_value" "test" {
  smart_class_parameter_id = "string"

  match = "hostgroup=Common"
  value = 8080
  omit  = false
}
galindro commented 3 years ago

The first one for sure, because I can dynamically repeat the blocks.

agriffit79 commented 2 years ago

I'm looking at this issue as I need smart class parameters for the project I am working on. I see some initial work was done on the override_values branch. The problem I see is you set value type to string, but it can also be int, bool, hash, array. Not sure if you had any thoughts on how to handle that?

lhw commented 2 years ago

That was one of the reasons I stopped working on it at the time. There is no good "AnyType" in the sdk api valuetype.go. So the only option would be to have a string representation in terraform plus an optional type field and on the foreman api side implement a union type plus some parsing for the field.

resource "foreman_smart_class_parameter_override" "test" {
  smart_class_parameter_id = "string"

  override {
    match = "fqdn=temp.yourdomain.net"
    value = 8080
    omit  = false
    type  = "int"
  }

  override {
    match = "hostgroup=Common"
    value = "8080"
    omit  = false
  }

  override {
    match = "hostgroup=Common"
    value = "[8080,\"8080\"]"
    omit  = false
    type  = "array"
  }
}

Seeing as this will turn ugly quick and we as team had no benefit from it at the time I chose not to invest time. If there is a good solution in a different provider this might be worth looking into otherwise I don't see any other good option.

agriffit79 commented 2 years ago

I did some research on this and the generally accepted approach seems to be to encode complex data structures in JSON. I also think it's not necessary to explicitly define the type as it's possible to infer it by attempting various type conversions and seeing which one succeeds.

I'm working on some PoC code now. Looks promising so far.

lhw commented 2 years ago

I did some research on this and the generally accepted approach seems to be to encode complex data structures in JSON. I also think it's not necessary to explicitly define the type as it's possible to infer it by attempting various type conversions and seeing which one succeeds.

Yes. Thats what i meant with a string representation. Doing it all with JSON is definitely a valid alternative. Fodoj did the same with complex types for compute_attributes in #35.

I'm working on some PoC code now. Looks promising so far.

Great. Thanks for your continued work on the provider :+1: