Skip to content

Conversation

@VishrutiBuddhadev
Copy link
Collaborator

No description provided.

VishrutiBuddhadev and others added 4 commits November 26, 2025 14:20
…open#265)

Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.43.0 to 0.45.0.
- [Commits](golang/crypto@v0.43.0...v0.45.0)

---
updated-dependencies:
- dependency-name: golang.org/x/crypto
  dependency-version: 0.45.0
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@@ -0,0 +1,16 @@
// Retrieve a specific DHCP Ipv6 Shared Network by filters
data "nios_dhcp_ipv6sharednetwork" "get_dhcp_ipv6sharednetwork_using_filters" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing .md files
Please run go generate and amend folder name if required !

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@@ -0,0 +1,16 @@
// Retrieve a specific DHCP Ipv6 Shared Network by filters
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Retrieve a specific DHCP Ipv6 Shared Network by filters
// Retrieve a specific DHCP IPv6 Shared Network by filters

Applies for all occurences

network1 := acctest.RandomIPv6Network()
network2 := acctest.RandomIPv6Network()
networks := []map[string]any{
{"ref": "${nios_ipam_ipv6network.test1.ref}"},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using Expand and Flatten Wrappers , cant we eliminate the use of "ref" somehow and just keep a string array of networks ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

// Ensure provider defined types fully satisfy framework interfaces.
var _ resource.Resource = &Ipv6sharednetworkResource{}
var _ resource.ResourceWithImportState = &Ipv6sharednetworkResource{}

Copy link
Collaborator

Choose a reason for hiding this comment

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

var _ resource.ResourceWithValidateConfig = &Ipv6sharednetworkResource{}

Default: booldefault.StaticBool(false),
MarkdownDescription: "Determines whether an IPv6 shared network is disabled or not. When this is set to False, the IPv6 shared network is enabled.",
},
"domain_name": schema.StringAttribute{
Copy link
Collaborator

Choose a reason for hiding this comment

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

IsValidFQDN required for this field ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Case Insensitive String detected !
Or validation against upper case !

"vendor_class": schema.StringAttribute{
Computed: true,
Optional: true,
Default: stringdefault.StaticString("DHCP"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Best not to keep a default for the field here !

},
MarkdownDescription: "If this field is set to True, the DHCP server generates a hostname and updates DNS with it when the DHCP client request does not contain a hostname.",
},
"ddns_server_always_updates": schema.BoolAttribute{
Copy link
Collaborator

Choose a reason for hiding this comment

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

ddns_use_option81 needs to be true for this field to work . Otherwise

image

Best add a validate Config

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Addressed all the review comments

},
MarkdownDescription: "Comment for the IPv6 shared network, maximum 256 characters.",
},
"ddns_domainname": schema.StringAttribute{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Case Insensitive String detected !
Or validation against upper case !

Default: booldefault.StaticBool(false),
MarkdownDescription: "Determines whether an IPv6 shared network is disabled or not. When this is set to False, the IPv6 shared network is enabled.",
},
"domain_name": schema.StringAttribute{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Case Insensitive String detected !
Or validation against upper case !

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds comprehensive support for IPv6 Shared Network resources and data sources to the Terraform NIOS provider, enabling management of DHCP IPv6 shared networks with extensive configuration options including DDNS settings, domain configurations, DHCP options, and extensible attributes.

Key Changes

  • Updates the infoblox-nios-go-client dependency to add Ipv6sharednetworkNetworks model with structured network references
  • Implements full CRUD operations for IPv6 shared networks with validation, import support, and extensible attributes handling
  • Adds comprehensive test coverage with 30+ test cases covering all configurable attributes and edge cases

Reviewed changes

Copilot reviewed 15 out of 20 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
vendor/modules.txt, go.mod, go.sum Updates infoblox-nios-go-client dependency to version with IPv6 shared network support
vendor/.../model_ipv6sharednetwork_networks.go New generated model for IPv6 shared network network references
vendor/.../model_ipv6sharednetwork.go Updates Networks field from []string to []Ipv6sharednetworkNetworks for structured references
internal/utils/utils.go Adds bounds checking to ExtractResourceRef to handle edge cases
internal/service/dhcp/model_ipv6sharednetwork.go Implements core model with Expand/Flatten functions and network reference handling
internal/service/dhcp/model_ipv6sharednetwork_options.go Implements DHCP options model with validation support
internal/service/dhcp/model_ipv6sharednetwork_logic_filter_rules.go Implements logic filter rules model
internal/service/dhcp/ipv6sharednetwork_resource.go Implements resource with CRUD operations, validation, and import support
internal/service/dhcp/ipv6sharednetwork_resource_test.go Comprehensive test suite with 30+ test cases
internal/service/dhcp/ipv6sharednetwork_data_source.go Implements data source with filtering and pagination
internal/service/dhcp/ipv6sharednetwork_data_source_test.go Data source tests for filters and extensible attributes
internal/service/dhcp/model_ipv6fixedaddresstemplate.go Adds Computed attribute to number_of_addresses and offset fields
internal/provider/provider.go Registers new resource and data source
examples/ and docs/ Adds documentation and usage examples

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

Comment on lines 336 to 337
// Check if ddns_use_option81 is set to false
if data.DdnsUseOption81.IsNull() && data.DdnsUseOption81.IsUnknown() && !data.DdnsUseOption81.ValueBool() {
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

Logic error in validation condition: The condition uses && (AND) when it should use || (OR). The current logic data.DdnsUseOption81.IsNull() && data.DdnsUseOption81.IsUnknown() && !data.DdnsUseOption81.ValueBool() can never be true because a value cannot be both null AND unknown AND have a false boolean value simultaneously. This should be data.DdnsUseOption81.IsNull() || data.DdnsUseOption81.IsUnknown() || !data.DdnsUseOption81.ValueBool() to properly check if ddns_use_option81 is not set to true.

Suggested change
// Check if ddns_use_option81 is set to false
if data.DdnsUseOption81.IsNull() && data.DdnsUseOption81.IsUnknown() && !data.DdnsUseOption81.ValueBool() {
// Check if ddns_use_option81 is not set to true
if data.DdnsUseOption81.IsNull() || data.DdnsUseOption81.IsUnknown() || !data.DdnsUseOption81.ValueBool() {

Copilot uses AI. Check for mistakes.
}
}

var options []Ipv6fixedaddresstemplateOptionsModel
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

Wrong type used in variable declaration: The variable options is declared as []Ipv6fixedaddresstemplateOptionsModel but should be []Ipv6sharednetworkOptionsModel since this is in the IPv6 shared network resource validation. This type mismatch will cause incorrect behavior when accessing option fields.

Suggested change
var options []Ipv6fixedaddresstemplateOptionsModel
var options []Ipv6sharednetworkOptionsModel

Copilot uses AI. Check for mistakes.
Comment on lines 696 to 697
resource.TestCheckResourceAttrPair(resourceName, "networks.0.ref", "nios_ipam_ipv6network.test1", "ref"),
resource.TestCheckResourceAttrPair(resourceName, "networks.1.ref", "nios_ipam_ipv6network.test2", "ref"),
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

Test assertion mismatch: The test is checking for networks.0.ref and networks.1.ref which suggests the networks field contains objects with a ref property. However, based on the schema and model definition, networks is defined as a List of String (line 250-258 in model_ipv6sharednetwork.go), not a list of objects. The correct assertion should be checking the string values directly: resource.TestCheckResourceAttrPair(resourceName, "networks.0", "nios_ipam_ipv6network.test1", "ref") without the .ref suffix on the networks field.

Copilot uses AI. Check for mistakes.
Check: resource.ComposeTestCheckFunc(
testAccCheckIpv6sharednetworkExists(context.Background(), resourceName, &v),
resource.TestCheckResourceAttr(resourceName, "networks.#", "1"),
resource.TestCheckResourceAttrPair(resourceName, "networks.0.ref", "nios_ipam_ipv6network.test1", "ref"),
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

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

Test assertion mismatch: Same issue as lines 696-697. The test is checking networks.0.ref but networks is a list of strings, not a list of objects. The correct assertion should be resource.TestCheckResourceAttrPair(resourceName, "networks.0", "nios_ipam_ipv6network.test1", "ref") without the .ref suffix.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

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