Add compatibility to "name" argument (#132)

* Add compatibility to "name" argument

The breaking change of making host.name read-only renders a lot of TF
configs unusable. This commit adds diff suppression handling to work
with name, shortname and fqdn even with older manifests.

* Remove deprecation of foreman_host.name attribute; extend README

The deprecation was meant to be an improvement but causes breaking
changes to many Terraform manifests. We removed the deprecation and
extended the README instead.

Author: bitkeks

README.md

@@ -2,6 +2,22 @@
 
 Terraform provider to interact with [Foreman](https://www.theforeman.org/).
 
+
+## Changes in 0.6.x
+Starting with `v0.6.0` some (breaking) changes require an update of Terraform manifests.
+
+* The host `build` argument was removed (`0.6.0`) and is replaced by `set_build_flag`. (`0.6.1`)
+  * The reason behind this change is complex and was thoroughly discussed in https://github.com/terraform-coop/terraform-provider-foreman/discussions/125
+  * Using the argument does one thing: it tells Foreman to set the `build` flag for a host. It defaults to `false`, setting it to `true` causes the host to be re-installed on next boot (network-based installation).
+* The `method` argument is re-introduced as `provision_method`. It can be either `build` (network-based) or `image` (image-based).
+  * Both options require different additional arguments, e.g the image to be used. See `examples/host/`.
+* The host `name` argument was considered for deprecation (`0.6.0`). 
+  * The `name` attribute has issues based on the "append_domain_name" setting in Foreman. It causes "inconsistent plan" errors when you give it a shortname as value, Terraform receives an FQDN back, and the `name` attribute is then used in variables in other places in your Terraform manifests.
+  * As an alternative, the `shortname` argument can be used instead. It is meant for the hostname without the domain part. If you use `name` as input argument, `shortname` will be filled by the provider automatically.
+  * To get the host's FQDN from the provider, use the read-only attribute `fqdn`. (`0.6.1`)
+  * **Use `shortname` and `fqdn` as variables in your manifests**! Example: `other_server = foreman_host.other_server.fqdn`. This will prevent you from running into inconsistent plans.
+
+
 ## Migration notice
 
 The provider has moved from its previous location at https://github.com/terraform-coop/terraform-provider-foreman

docs/resources/foreman_host.md

@@ -18,6 +18,7 @@ resource "foreman_host" "example" {
 
 The following arguments are supported:
 
+- `architecture_id` - (Optional) ID of the architecture of this host
 - `bmc_success` - (Optional) REMOVED - Tracks the partial state of BMC operations on host creation. If these operations fail, the host will be created in Foreman and this boolean will remain `false`. On the next `terraform apply` will trigger the host update to pick back up with the BMC operations.
 - `comment` - (Optional) Add additional information about this host.Note: Changes to this attribute will trigger a host rebuild.
 - `compute_attributes` - (Optional) Hypervisor specific VM options. Must be a JSON string, as every compute provider has different attributes schema
@@ -34,21 +35,26 @@ The following arguments are supported:
 - `managed` - (Optional) Whether or not this host is managed by Foreman. Create host only, don't set build status or manage power states.
 - `medium_id` - (Optional, Force New) ID of the medium mounted on the host.
 - `model_id` - (Optional) ID of the hardware model if applicable
+- `name` - (Optional, Force New) Name of this host as stored in Foreman. Can be short name or FQDN, depending on your Foreman settings (especially the setting 'append_domain_name_for_hosts').
 - `operatingsystem_id` - (Optional, Force New) ID of the operating system to put on the host.
 - `owner_id` - (Optional) ID of the user or usergroup that owns the host.
 - `owner_type` - (Optional) Owner of the host, must be either User ot Usergroup
 - `parameters` - (Optional) A map of parameters that will be saved as host parameters in the machine config.
 - `provision_method` - (Optional, Force New) Sets the provision method in Foreman for this host: either network-based ('build') or image-based ('image')
+- `ptable_id` - (Optional) ID of the partition table the host should use
 - `puppet_class_ids` - (Optional) IDs of the applied puppet classes.
 - `retry_count` - (Optional) Number of times to retry on a failed attempt to register or delete a host in foreman.
+- `root_password` - (Optional) Default root password
 - `set_build_flag` - (Optional) Sets the Foreman-internal 'build' flag on this host - even if it is already built completely.
-- `shortname` - (Required, Force New) The short name of this host. Example: when the FQDN is 'host01.example.org', then 'host01' is the short name.
+- `shortname` - (Optional, Force New) The short name of this host. Example: when the FQDN is 'host01.example.org', then 'host01' is the short name.
+- `subnet_id` - (Optional) ID of the subnet the host should be placed in
 
 
 ## Attributes Reference
 
 The following attributes are exported:
 
+- `architecture_id` - ID of the architecture of this host
 - `comment` - Add additional information about this host.Note: Changes to this attribute will trigger a host rebuild.
 - `compute_attributes` - Hypervisor specific VM options. Must be a JSON string, as every compute provider has different attributes schema
 - `compute_profile_id` - 
@@ -72,9 +78,12 @@ The following attributes are exported:
 - `owner_type` - Owner of the host, must be either User ot Usergroup
 - `parameters` - A map of parameters that will be saved as host parameters in the machine config.
 - `provision_method` - Sets the provision method in Foreman for this host: either network-based ('build') or image-based ('image')
+- `ptable_id` - ID of the partition table the host should use
 - `puppet_class_ids` - IDs of the applied puppet classes.
 - `retry_count` - Number of times to retry on a failed attempt to register or delete a host in foreman.
+- `root_password` - Default root password
 - `set_build_flag` - Sets the Foreman-internal 'build' flag on this host - even if it is already built completely.
 - `shortname` - The short name of this host. Example: when the FQDN is 'host01.example.org', then 'host01' is the short name.
+- `subnet_id` - ID of the subnet the host should be placed in
 - `token` - Build token. Can be used to signal to Foreman that a host build is complete.
 

foreman/api/host.go

@@ -51,7 +51,7 @@ type ForemanHost struct {
 
 	// Shortname, FQDN without DomainName.
 	// Not provided by the API, only available in templates with embedded Ruby
-	Shortname string
+	Shortname string `json:"-"`
 
 	// The "fqdn" field in the Terraform schema exists only in the schema,
 	// not in this struct. Reason: name is a Foreman-managed field that can either hold

foreman/resource_foreman_host.go

@@ -329,19 +329,21 @@ func resourceForemanHost() *schema.Resource {
 			},
 
 			"name": {
-				Type:        schema.TypeString,
-				Computed:    true,
-				Optional:    false,
-				Required:    false,
-				Deprecated:  "[Deprecated. Use 'shortname' instead!]",
-				Description: "Name of this host as stored in Foreman. Can be short name or FQDN, depending on your Foreman settings (especially the setting 'append_domain_name_for_hosts').",
+				Type:             schema.TypeString,
+				Computed:         true,
+				Optional:         true,
+				ForceNew:         true,
+				Required:         false,
+				Description:      "Name of this host as stored in Foreman. Can be short name or FQDN, depending on your Foreman settings (especially the setting 'append_domain_name_for_hosts').",
+				DiffSuppressFunc: resourceForemanHostNameDiffSuppressFunc,
 			},
 
 			"shortname": {
 				Type:        schema.TypeString,
 				ForceNew:    true,
-				Optional:    false,
-				Required:    true,
+				Computed:    true,
+				Optional:    true,
+				Required:    false,
 				Description: "The short name of this host. Example: when the FQDN is 'host01.example.org', then 'host01' is the short name.",
 				ValidateDiagFunc: func(value interface{}, p cty.Path) diag.Diagnostics {
 					var diags diag.Diagnostics
@@ -355,6 +357,16 @@ func resourceForemanHost() *schema.Resource {
 					}
 					return diags
 				},
+				DiffSuppressFunc: func(k, oldValue, newValue string, d *schema.ResourceData) bool {
+					name := d.Get("name").(string)
+
+					// Use case: manifest has no "shortname" defined, but uses "name". Then an apply fetches the shortname,
+					// but a new apply would think it should be re-set to "".
+					if name != "" && newValue == "" && strings.HasPrefix(name, oldValue) {
+						return true
+					}
+					return false
+				},
 			},
 
 			"fqdn": {
@@ -1418,3 +1430,43 @@ func resourceForemanHostCustomizeDiffComputeAttributes(ctx context.Context, d *s
 	d.SetNew("compute_attributes", flattenComputeAttributes(oldMap))
 	return nil
 }
+
+func resourceForemanHostNameDiffSuppressFunc(k, oldValue, newValue string, d *schema.ResourceData) bool {
+	domainName := d.Get("domain_name").(string)
+	if domainName == "" {
+		// If domainName not given, no comparison is possible
+		return false
+	}
+
+	beforeOld, afterOld, foundOld := strings.Cut(oldValue, ".")
+	beforeNew, afterNew, foundNew := strings.Cut(newValue, ".")
+
+	if !foundOld && foundNew {
+		if oldValue == beforeNew {
+			// Shortname was expanded to FQDN
+			return true
+		}
+
+		if afterNew == domainName {
+			return true
+		}
+	}
+
+	if (!foundOld && !foundNew) && (beforeOld != beforeNew) {
+		// Neither value has a dot but values differ, so no suppression
+		return false
+	}
+
+	if (foundOld && foundNew) && (afterOld != afterNew) {
+		// Domains differ
+		return false
+	}
+
+	// Use case: We pass in a shortname, Foreman expands it and the next "terraform apply"
+	// thinks the shortname is a new value.
+	if (foundOld && !foundNew) && strings.HasPrefix(oldValue, newValue) {
+		return true
+	}
+
+	return false
+}

mkdocs.yml

@@ -44,6 +44,7 @@ nav:
     - 'foreman_partitiontable': 'data-sources/foreman_partitiontable.md'
     - 'foreman_provisioningtemplate': 'data-sources/foreman_provisioningtemplate.md'
     - 'foreman_puppetclass': 'data-sources/foreman_puppetclass.md'
+    - 'foreman_setting': 'data-sources/foreman_setting.md'
     - 'foreman_smartclassparameter': 'data-sources/foreman_smartclassparameter.md'
     - 'foreman_smartproxy': 'data-sources/foreman_smartproxy.md'
     - 'foreman_subnet': 'data-sources/foreman_subnet.md'