Skip to content

[FIX][NPA-1847] - Added a validation on node_info field #523

Open
Chaithra001 wants to merge 7 commits into
infobloxopen:mainfrom
Chaithra001:NPA-1847
Open

[FIX][NPA-1847] - Added a validation on node_info field #523
Chaithra001 wants to merge 7 commits into
infobloxopen:mainfrom
Chaithra001:NPA-1847

Conversation

@Chaithra001

@Chaithra001 Chaithra001 commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Issue: When node_info contained two nodes and enable_ha was set to false, the WAPI response returned only a single node because the member was treated as a standalone node and mgmt_lan information was also not returned.

Fix: Added validation to ensure that enable_ha is set to true whenever node_info contains two nodes, preventing invalid configurations and ensuring the expected node and mgmt_lan data are returned.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a configuration-time guard in the Grid Member resource to prevent invalid standalone configurations that specify an HA-style node_info (2 nodes) while enable_ha is false, which previously led to incomplete/incorrect WAPI results (single node returned and missing mgmt_lan).

Changes:

  • Add ValidateConfig validation requiring enable_ha = true when node_info contains exactly 2 nodes.
  • Surface a clear validation error message to block the invalid configuration before WAPI interaction.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +559 to +564

if len(nodeInfo) == 2 {
if data.EnableHa.IsNull() || data.EnableHa.IsUnknown() || !data.EnableHa.ValueBool() {
resp.Diagnostics.AddError("Validation Error", "enable_ha must be true when node_info contains 2 nodes")
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix the formatting issue

Comment on lines +560 to +563
if len(nodeInfo) == 2 {
if data.EnableHa.IsNull() || data.EnableHa.IsUnknown() || !data.EnableHa.ValueBool() {
resp.Diagnostics.AddError("Validation Error", "enable_ha must be true when node_info contains 2 nodes")
}
Comment on lines +559 to +564

if len(nodeInfo) == 2 {
if data.EnableHa.IsNull() || data.EnableHa.IsUnknown() || !data.EnableHa.ValueBool() {
resp.Diagnostics.AddError("Validation Error", "enable_ha must be true when node_info contains 2 nodes")
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix the formatting issue

if len(nodeInfo) == 2 {
if data.EnableHa.IsNull() || data.EnableHa.IsUnknown() || !data.EnableHa.ValueBool() {
resp.Diagnostics.AddError("Validation Error", "enable_ha must be true when node_info contains 2 nodes")
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add test case around this

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}
}

if len(nodeInfo) == 2 {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible len(nodeInfo) > 2, if yes then write the logic accordingly

),
},
// Update and Read
{

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add test cases for other failure scenarios too

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@vchandrajha vchandrajha left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGME

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