[MGMT-21038] Add hostName to host - used to manage inventory hostname. #10093

coderabbitai · 2026-03-31T21:33:59Z

⚠️ Potential issue | 🟠 Major

Preserve user-set RequestedHostname when inventory hostname is missing

On Line 4710, host.RequestedHostname is overwritten whenever host.Hostname is empty. That can clobber a user-updated requested hostname before inventory arrives, which conflicts with the hostname-separation goal.

Proposed fix

- if host.Hostname == "" { + if host.Hostname == "" && host.RequestedHostname == "" { host.Hostname = host.ID.String() host.RequestedHostname = host.ID.String() }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@internal/bminventory/inventory.go` around lines 4708 - 4711, The current logic sets host.RequestedHostname unconditionally when host.Hostname is empty, which overwrites any user-provided requested hostname; change it so host.Hostname is set to host.ID.String() if empty, but only set host.RequestedHostname to host.ID.String() if host.RequestedHostname is also empty—i.e., update the block around host.Hostname and host.RequestedHostname so host.RequestedHostname is preserved when already set (referencing host.Hostname and host.RequestedHostname).

coderabbitai · 2026-03-31T21:33:59Z

⚠️ Potential issue | 🟠 Major

Do not overwrite persisted inventory hostname with host ID when inventory hostname is empty.

On Line 412-Line 414, the fallback goes straight to h.ID.String(). With Line 425 persisting that value, a transient empty inventory.Hostname can erase a previously learned inventory hostname, which conflicts with the preservation goal of this PR.

Proposed fix

- var hostName string = inventory.Hostname + var hostName string = inventory.Hostname //in case RequestedHostname already set from UpdateHostname, can not change it from inventory if hostName == "" { - hostName = h.ID.String() + hostName = h.Hostname + } + if hostName == "" { + hostName = h.ID.String() } if h.RequestedHostname == "" { h.RequestedHostname = hostName }

Also applies to: 425-426

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@internal/host/host.go` around lines 410 - 417, The current logic assigns hostName = h.ID.String() whenever inventory.Hostname is empty, which allows a transient empty inventory.Hostname to overwrite a previously persisted hostname; change the logic in the block that sets hostName and h.RequestedHostname so that you only fall back to h.ID.String() if both inventory.Hostname and h.RequestedHostname are empty (i.e., preserve any existing h.RequestedHostname/persisted hostname), and ensure the persistence code that writes h.RequestedHostname does not save the h.ID.String() fallback when a persisted hostname already exists.

coderabbitai · 2026-03-31T21:33:59Z

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash set -euo pipefail echo "== Check for host hostname schema migration/backfill ==" fd -i '*.sql' | xargs -r rg -n -i 'alter\s+table.*host|add\s+column.*hostname|update.*host.*set.*hostname' rg -n --iglob '*.go' 'AutoMigrate|Migrate|AddColumn|hostname' echo echo "== Check Hostname assignment paths ==" rg -n --iglob '*.go' '\.Hostname\s*=|Hostname\s*:' rg -n --iglob '*.go' '\.RequestedHostname\s*=|RequestedHostname\s*:' echo echo "== Audit GetCurrentHostName call sites for hard-fail behavior ==" rg -n --iglob '*.go' '\bGetCurrentHostName\s*\(' -C 3

Repository: openshift/assisted-service

Length of output: 427

🏁 Script executed:

#!/bin/bash set -euo pipefail echo "== Check for SQL migration files and hostname-related changes ==" find . -name "*.sql" -type f | head -20 echo echo "== Search for migrations in Go code ==" rg -n 'AutoMigrate|Migrate.*Host|AddColumn.*hostname' --type go | head -30 echo echo "== Search for Hostname field assignments ==" rg -n '\.Hostname\s*=' --type go -B 2 -A 2 | head -50 echo echo "== Search for RequestedHostname assignments ==" rg -n '\.RequestedHostname\s*=' --type go -B 2 -A 2 | head -50 echo echo "== Find all GetCurrentHostName call sites ==" rg -n 'GetCurrentHostName\s*\(' --type go -B 2 -A 3

Repository: openshift/assisted-service

Length of output: 12692

🏁 Script executed:

#!/bin/bash echo "== Host model definition ==" rg -n 'type Host struct' --type go -A 50 | head -80 echo echo "== Context around inventory.go backfill (lines 4700-4720) ==" sed -n '4700,4720p' internal/bminventory/inventory.go echo echo "== Full GetCurrentHostName function ==" sed -n '47,56p' internal/host/hostutil/host_utils.go echo echo "== GetHostnameForMsg function (shows the fallback) ==" sed -n '58,65p' internal/host/hostutil/host_utils.go echo echo "== When/where is the inventory.go code (4708-4711) called? ==" rg -n 'if host.Hostname == ""' --type go -B 5 -A 5

Repository: openshift/assisted-service

Length of output: 9656

🏁 Script executed:

#!/bin/bash echo "== Check Inventory field type and usage ==" rg -n 'Inventory.*string' --type go -B 2 -A 2 | grep -A 3 'type Host' echo echo "== Search for UnmarshalInventory (mentioned in suggested patch) ==" rg -n 'UnmarshalInventory' --type go -B 2 -A 10 | head -50 echo echo "== Check for any hostname extraction from Inventory JSON ==" rg -n 'inventory.*[Hh]ostname|[Hh]ostname.*inventory' --type go -i -B 3 -A 3 | head -80 echo echo "== Check when customizeHost is called in the flow ==" rg -n 'customizeHost' --type go -B 2 -A 2

Repository: openshift/assisted-service

Length of output: 17890

🏁 Script executed:

#!/bin/bash echo "== Check when ignition generation happens and flow to GetCurrentHostName ==" rg -n 'GenerateIgnitionManifest|generateIgnitionManifest' --type go -B 5 -A 5 | head -100 echo echo "== Check where GetCurrentHostName is called - full flow context ==" rg -n 'GetCurrentHostName' --type go -B 10 -A 5 | head -150 echo echo "== Check if Hostname field is set in any pre-processing before ignition ==" rg -n 'host.Hostname\s*=' --type go -B 3 -A 1 | grep -v vendor | head -60

Repository: openshift/assisted-service

Length of output: 5236

🏁 Script executed:

#!/bin/bash echo "== Check git history for GetCurrentHostName changes ==" git log --oneline -20 -- internal/host/hostutil/host_utils.go 2>/dev/null | head -10 echo echo "== Check git diff to see what was changed ==" git show HEAD:internal/host/hostutil/host_utils.go 2>/dev/null | sed -n '47,56p' echo echo "== Check if any test covers the error case ==" rg -n 'GetCurrentHostName' --type go test | head -30 echo echo "== Check updateInventory in host.go (mentioned in comment) ==" rg -n 'updateInventory' --type go -B 3 -A 10 | grep -A 8 'func.*updateInventory'

Repository: openshift/assisted-service

Length of output: 3187

🏁 Script executed:

#!/bin/bash echo "== Look at the full git commit that made this change ==" git show faaaf6ca --stat 2>/dev/null | head -30 echo echo "== Check where ignition gets host object from ==" rg -n 'writeSingleHostFile|SetHostnameForNodeIgnition' --type go -B 15 | grep -A 15 'func.*' | head -100 echo echo "== Check if host is fetched from DB before ignition generation ==" rg -n 'GetHost|db.First|db.Where.*Host' --type go internal/ignition/ -A 3 | head -80 echo echo "== Verify what GetHostByID or similar returns ==" rg -n 'func.*GetHost' --type go internal/host/ -A 15 | head -100

Repository: openshift/assisted-service

Length of output: 16340

🏁 Script executed:

#!/bin/bash echo "== Check who calls GenerateIgnitionManifests/GenerateInstallManifests ==" rg -n 'GenerateInstallManifests|GenerateIgnitionManifests' --type go -B 5 | grep -v test | head -100 echo echo "== Check if hosts are loaded fresh from DB before ignition generation ==" rg -n 'GetCluster.*Hosts|GetClusterHosts|cluster.Hosts' --type go internal/ignition/ -B 3 -A 3 | head -80 echo echo "== Look at the actual test to see host setup ==" sed -n '1067,1085p' internal/ignition/installmanifests_test.go

Repository: openshift/assisted-service

Length of output: 7839

🏁 Script executed:

#!/bin/bash echo "== Check Host model struct for Hostname field definition ==" rg -n 'type Host struct' --type go -A 100 vendor/github.com/openshift/assisted-service/models/host.go | grep -i 'hostname\|inventory' | head -20 echo echo "== Check if Hostname has a default value or is explicitly backfilled ==" rg -n 'host.Hostname' --type go internal/ | grep -v test | grep -v vendor | head -40 echo echo "== Check when ignition generation is triggered and from where ==" rg -n 'NewGenerator|GenerateInstallManifests' --type go -B 3 -A 3 | grep -v test | head -80 echo echo "== Check if there's any code that explicitly sets Hostname before ignition ==" rg -n 'host.Hostname\s*=' --type go internal/ | grep -v test | grep -v vendor

Repository: openshift/assisted-service

Length of output: 2850

🏁 Script executed:

#!/bin/bash echo "== Check where cluster is loaded before passing to generator ==" rg -n 'NewGenerator\(' --type go -B 20 | head -150 echo echo "== Check if cluster hosts are guaranteed to go through customizeHost ==" rg -n 'GetCluster\|GetClusterByID' --type go internal/ -A 10 | grep -v test | grep -v vendor | head -100 echo echo "== Check the generator.Generate method to see if it loads fresh cluster ==" sed -n '190,250p' internal/ignition/installmanifests.go

Repository: openshift/assisted-service

Length of output: 14931

🏁 Script executed:

#!/bin/bash echo "== Check who calls the generator from pkg/generator/generator.go ==" rg -n 'Generate\(' --type go pkg/generator/ -B 5 -A 3 | head -80 echo echo "== Check the flow in main or cmd that calls this ==" rg -n 'generator.Generate\|GenerateIgnitionManifests' --type go cmd/ -B 5 -A 3 | head -100 echo echo "== Check if there's any GetCluster call that goes through customizeHost before generator ==" rg -n 'GetCluster|cluster :=' --type go pkg/generator/ -B 3 -A 3

Repository: openshift/assisted-service

Length of output: 1059

🏁 Script executed:

#!/bin/bash echo "== Search for the caller of pkg/generator/generator.go Generate ==" rg -n 'generator.GenerateIgnitionManifests\|GenerateAssets' --type go -B 10 | head -150 echo echo "== Check if hosts are loaded fresh from DB in the ignition generation flow ==" rg -n 'GetClusterHosts\|cluster.Hosts\s*=' --type go pkg/ internal/ | grep -v test | grep -v vendor | head -60 echo echo "== Look for the entry point that triggers ignition generation ==" rg -n 'func.*Generate.*ignition\|func.*GenerateAssets' --type go -B 5 | grep -v test | head -100

Repository: openshift/assisted-service

Length of output: 281

🏁 Script executed:

#!/bin/bash echo "== Check previous versions of GetCurrentHostName ==" git log --oneline -30 -- internal/host/hostutil/host_utils.go 2>/dev/null echo echo "== Show the actual changes in the commit ==" git show faaaf6ca:internal/host/hostutil/host_utils.go 2>/dev/null | sed -n '40,70p' echo echo "== Check the parent commit version ==" git show faaaf6ca^:internal/host/hostutil/host_utils.go 2>/dev/null | sed -n '40,70p' echo echo "== Verify if UnmarshalInventory exists and can extract hostname ==" rg -n 'func UnmarshalInventory' --type go -A 20 internal/common/

Repository: openshift/assisted-service

Length of output: 1772

🏁 Script executed:

#!/bin/bash echo "== Check git log before this commit to find GetCurrentHostName ==" git log --all --oneline -- internal/host/hostutil/host_utils.go 2>/dev/null | tail -20 echo echo "== Show file content at HEAD for context ==" git show HEAD:internal/host/hostutil/host_utils.go 2>/dev/null | sed -n '45,65p' echo echo "== Verify what callers expect - check error handling ==" rg -n 'GetCurrentHostName' --type go -B 5 -A 5 | grep -E '(wraps|wrap|Wrapf|errors.Wrap)'

Repository: openshift/assisted-service

Length of output: 2417

🏁 Script executed:

#!/bin/bash echo "== Check updateInventory in host.go (lines 305-400) ==" sed -n '305,400p' internal/host/host.go echo echo "== Look for where Hostname is set from inventory ==" rg -n 'inventory.Hostname' --type go -B 3 -A 3 internal/host/

Repository: openshift/assisted-service

Length of output: 5765

🏁 Script executed:

#!/bin/bash echo "== Check the Host model commit to see when Hostname was added ==" git log --all --oneline --grep="Hostname\|hostname" -- models/host.go 2>/dev/null | head -10 echo echo "== Check if Hostname has a database migration ==" git show faaaf6ca -- internal/migrations/ 2>/dev/null | head -100 echo echo "== Verify the exact change in the Host model ==" git show faaaf6ca -- vendor/github.com/openshift/assisted-service/models/host.go 2>/dev/null | grep -A 5 -B 5 'Hostname string' echo echo "== Check if the Hostname field is initialized anywhere by default ==" rg -n 'Hostname.*=.*""' --type go internal/ | grep -v test | grep -v vendor

Repository: openshift/assisted-service

Length of output: 5594

🏁 Script executed:

#!/bin/bash echo "== Check where generator is called from - trace back to cluster source ==" rg -n 'GenerateAssets\|Generate\(' --type go pkg/generator/ internal/ignition/ -B 20 | grep -E '(func|cluster|GetCluster|db.First)' | head -100 echo echo "== Check if there's direct DB loading that bypasses customizeHost ==" rg -n 'db.Preload.*Host|db.First.*Cluster.*Host' --type go internal/ | grep -v test | grep -v vendor | head -20 echo echo "== Verify ignition generation entry point gets cluster from API or DB ==" rg -n 'func.*Generate.*context\|func.*GenerateAssets' --type go cmd/ pkg/ -A 20 | head -80

Repository: openshift/assisted-service

Length of output: 680

Add inventory hostname fallback for defensive robustness.

While host.Hostname is typically populated via customizeHost() in API response paths, adding a fallback to host.Inventory when both RequestedHostname and Hostname are empty would provide better defensiveness against edge cases. The current code returns an error if both fields are empty, but hosts in the Inventory field may still contain valid hostname data. Consider adding the suggested compatibility patch to extract hostname from the unpacked inventory as a final fallback before returning an error, ensuring no valid hostname data is discarded.

Suggested defensive fallback patch

func GetCurrentHostName(host *models.Host) (string, error) { if host.RequestedHostname != "" { return host.RequestedHostname, nil } - // in case inventory Hostname was not set. - if host.Hostname == "" { - return "", errors.Errorf("hostname is empty") + if host.Hostname != "" { + return host.Hostname, nil } - return host.Hostname, nil + // Fallback for hosts not yet backfilled with Hostname field. + inventory, err := common.UnmarshalInventory(host.Inventory) + if err == nil && inventory != nil && inventory.Hostname != "" { + return inventory.Hostname, nil + } + return "", errors.Errorf("hostname is empty") }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@internal/host/hostutil/host_utils.go` around lines 51 - 55, The code currently returns an error when host.Hostname is empty; update the hostname resolution logic to try host.RequestedHostname first, then host.Hostname, and as a final defensive fallback attempt to extract a hostname from host.Inventory (the unpacked inventory JSON) before returning an error; locate the hostname check around the Hostname empty-return block and add logic to unmarshal host.Inventory (or use existing inventory parsing helpers) to read a hostname field and return it if present, otherwise keep returning the error.

-Original file line number
+Diff line change
@@ Expand Up @@
     		isSno = cluster.ControlPlaneCount == 1
     	}
     	host.ProgressStages = b.hostApi.GetStagesByRole(host, isSno)
-    	host.RequestedHostname = hostutil.GetHostnameForMsg(host)
+    	// in case inventory hostname was not set, HostName set in host.go updateInventory
+    	if host.Hostname == "" {
+    		host.Hostname = host.ID.String()
+    		host.RequestedHostname = host.ID.String()
+    	}
     }
     func proxySettingsChanged(params *models.V2ClusterUpdateParams, cluster *common.Cluster) bool {
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up @@
     		installationDiskPath = common.GetDeviceFullName(installationDisk)
     		installationDiskID = common.GetDeviceIdentifier(installationDisk)
     	}
+    	var hostName string = inventory.Hostname
+    	//in case RequestedHostname already set from UpdateHostname, can not change it from inventory
+    	if hostName == "" {
+    		hostName = h.ID.String()
+    	}
+    	if h.RequestedHostname == "" {
+    		h.RequestedHostname = hostName
+    	}
     	disksToBeFormatted := strings.Join(common.GetDisksIdentifiersToBeFormatted(inventory), ",")
     	// If there is substantial change in the inventory that might cause the state machine to move to a new status
     	// or one of the validations to change, then the updated_at field has to be modified.  Otherwise, we just
     	// perform update with touching the updated_at field
     	updates := map[string]interface{}{
+    		"Hostname":               hostName,
+    		"RequestedHostname":      h.RequestedHostname,
     		"inventory":              inventoryStr,
     		"installation_disk_path": installationDiskPath,
     		"installation_disk_id":   installationDiskID,
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -4926,6 +4926,8 @@ definitions: @@
             type: string
           requested_hostname:
             type: string
+          hostname:
+            type: string
           user_name:
             type: string
           media_status:
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MGMT-21038] Add hostName to host - used to manage inventory hostname. #10093

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

coderabbitai bot Mar 31, 2026

Uh oh!

coderabbitai bot Mar 31, 2026

Uh oh!

coderabbitai bot Mar 31, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

-Original file line number
+Diff line change
@@ Expand Up / @@ -45,15 +45,14 @@ var ForbiddenHostnames = []string{ @@
     }
     func GetCurrentHostName(host *models.Host) (string, error) {
-    	var inventory models.Inventory
     	if host.RequestedHostname != "" {
     		return host.RequestedHostname, nil
     	}
-    	err := json.Unmarshal([]byte(host.Inventory), &inventory)
-    	if err != nil {
-    		return "", err
+    	// in case inventory Hostname was not set.
+    	if host.Hostname == "" {
+    		return "", errors.Errorf("hostname is empty")
     	}
-    	return inventory.Hostname, nil
+    	return host.Hostname, nil
     }
     func GetHostnameForMsg(host *models.Host) string {
@@ Expand Down @@

[MGMT-21038] Add hostName to host - used to manage inventory hostname. #10093

Are you sure you want to change the base?

Uh oh!

[MGMT-21038] Add hostName to host - used to manage inventory hostname. #10093

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!