Add Host resource #5
Conversation
Signed-off-by: Alina Buzachis <[email protected]>
Signed-off-by: Alina Buzachis <[email protected]>
Signed-off-by: Alina Buzachis <[email protected]>
alinabuzachis
force-pushed
the
host_resource
branch
2 times, most recently
from
ff59533 to
4a2f37a
Compare
Signed-off-by: Alina Buzachis <[email protected]>
alinabuzachis
force-pushed
the
host_resource
branch
from
4a2f37a to
832f0a8
Compare
Signed-off-by: Alina Buzachis <[email protected]>
There was a problem hiding this comment.
I don't know if it should be part of this PR (@gravesm?), but we will need to add ID as an attribute to the Group resource so someone can create a group and use that group's ID to add it to a host all within a single terraform config file.
| }, | ||
| "instance_id": schema.StringAttribute{ | ||
| Optional: true, | ||
| Computed: true, |
There was a problem hiding this comment.
I don't think this should be computed, only optional. From the Terraform side, if this isn't provided it will just be null. Same for description.
There was a problem hiding this comment.
instance_id is computed because, supposing the user does not set it, so, it will be passed as nil, it will be returned by the api as an empty string. Same for description. This is my experience and understanding.
{
....
},
"created": "2024-01-18T16:54:45.198378Z",
"modified": "2024-01-18T16:54:45.198387Z",
"name": "tf_host",
"description": "",
"inventory": 1,
"enabled": true,
"instance_id": "",
"variables": "",
"has_active_failures": false,
"has_inventory_sources": false,
"last_job": null,
"last_job_host_summary": null,
"ansible_facts_modified": null
}
There was a problem hiding this comment.
Yes from the API side it's an empty string but from the Terraform config perspective it can just remain null.
| stringplanmodifier.UseStateForUnknown(), | ||
| }, | ||
| }, | ||
| "variables": schema.StringAttribute{ |
There was a problem hiding this comment.
This should have a CustomType: jsontypes.NormalizedType{} like it does in the other resources so it will validate JSON strings.
There was a problem hiding this comment.
I've followed what group_resource has defined as types for variables https://github.com/ansible/terraform-provider-aap/blob/main/internal/provider/group_resource.go#L66 I can fix that, but let's try to be consistent then.
There was a problem hiding this comment.
Oooh I totally missed that in the group resource! Yes we should update it there too!
| Computed: true, | ||
| Description: "Defaults true.", | ||
| }, | ||
| "group_id": schema.Int64Attribute{ |
There was a problem hiding this comment.
I think this should be a list so the host can be associated with multiple groups.
There was a problem hiding this comment.
The API requires it to be an integer. In any case, as explained in the other comments, I can allow the user to specify different groups
"groups": schema.ListNestedAttribute{
NestedObject: schema.NestedAttributeObject{
Attributes: map[string]schema.Attribute{
"id": schema.Int64Attribute{
Required: true,
},
"disassociate": schema.BoolAttribute{
Optional: true,
},
},
},
Optional: true,
},
},
but we will basically pass to the api an id: int and an optional disassociate flag.
| Optional: true, | ||
| Description: "Set this option to associate an existing group with a host.", | ||
| }, | ||
| "disassociate_group": schema.BoolAttribute{ |
There was a problem hiding this comment.
I don't think this is necessary. Once the associated groups are a list, user should be able to disassociate a group from the host just by removing the group from its list of group IDs in the terraform config.
There was a problem hiding this comment.
A group is not removed from a host unless disassociate parameter is passed. See https://docs.ansible.com/automation-controller/latest/html/controllerapi/api_ref.html#/Hosts/Hosts_hosts_groups_create Also, id must be passed as an integer and a list is not accepted, this is my understanding.
I can implement something like:
groups = [
{
"id" : 2
},
{
"id": 3
"disassociate": true
}
]
}
It will mainly make a group association for every id.
There was a problem hiding this comment.
We need to do the work of figuring out which groups need to be added/removed on our side. We'll need to list the groups a host is a member of, compare that to what's in the tf config, and then add and remove as necessary. The terraform config should look something like:
resource "aap_host" "foo_host" {
...
groups = [group_a.id, group_b.id]
}
resource "aap_group" "group_a" {}
resource "aap_group" "group_b" {}
| fmt.Sprintf("expected (%d) got (%s)", http.StatusCreated, resp.Status)) | ||
| return diags | ||
| } | ||
| diags.Append(IsResponseValid(resp, err, http.StatusCreated)...) |
There was a problem hiding this comment.
This function no longer stops execution when a fatal error was reached. You should check if the diagnostics object has errors before continuing: https://developer.hashicorp.com/terraform/plugin/framework/diagnostics#haserror
Let's add it to this PR. |
| // and append the URL from the state data to the plan data. | ||
| resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...) | ||
| resp.Diagnostics.Append(req.State.Get(ctx, &data_with_URL)...) | ||
| data.URL = data_with_URL.URL |
There was a problem hiding this comment.
This is not needed. The URL will be retrieved from the state and populated in the plan.
|
Close in favor of #6 |
|
Why open a new PR? |
Because I messed up the branch. |
Add Host resource