Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #622 +/- ##
==========================================
- Coverage 29.42% 21.26% -8.17%
==========================================
Files 246 252 +6
Lines 28407 28537 +130
==========================================
- Hits 8360 6068 -2292
- Misses 19893 22243 +2350
- Partials 154 226 +72 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
The best way to advance this PR will be to split out the helper refactorings into separate (easy to review) PRs. This will prevent the creeping in of new code with the patterns we want to move away from. |
Extracted from #622 Also ran goimports for clean up (triggered by stray imports based test failure)
d6b13e7 to
f3b96d7
Compare
This PR moves the helper functions from provider.go to a new comparisons package. This package will be used by provider, tests, and resources. The refactoring will assist with splitting resources into resource specific packages. #106 The previous helper functions have been deprecated and will be removed in a future commit. Extracted from #622 (PR grew to large to maintain. Refactoring is not a priority and rebasing gets hard as the underlying code changes) Related to #674 Related to #654 Signed-off-by: Marques Johansson <mjohansson@equinix.com>
f3b96d7 to
20a6b6b
Compare
|
@ctreatma @equinix/governor-ne-network-edge-engineering I updated this refactor branch and The biggest trick was moving the There are a few more rounds of cleanup that could happen to make this PR easier to review:
|
| // from https://stackoverflow.com/a/45428032 | ||
| func Difference(a, b []string) []string { | ||
| mb := make(map[string]struct{}, len(b)) | ||
| // Difference returns the elements in `a` that aren't in `b`. |
There was a problem hiding this comment.
making Difference use generics doesn't have to be done in this PR.
|
#720 should be merged ahead of this so this PR can rebase and apply the sweeper related changes from that PR |
8709181 to
2404fa2
Compare
|
After the latest rebase to pickup sweeper changes, and some changes to use SetMap (and a diag equivalent) for which more will be needed, there are new errors: I'm wondering if the init() calls in acceptance_test.go are not initializing the provider list correctly for cross-package testing. |
7fb2dce to
921c699
Compare
Remove isStringInSlice in favor of slices.Contains. Extracted from equinix#622 Related to equinix#665 Signed-off-by: Marques Johansson <mjohansson@equinix.com>
This PR moves the helper functions from provider.go to a new comparisons package. This package will be used by provider, tests, and resources. The refactoring will assist with splitting resources into resource specific packages. equinix#106 The previous helper functions have been deprecated and will be removed in a future commit. Extracted from equinix#622 (PR grew to large to maintain. Refactoring is not a priority and rebasing gets hard as the underlying code changes) Related to equinix#674 Related to equinix#654 Signed-off-by: Marques Johansson <mjohansson@equinix.com>
Signed-off-by: Marques Johansson <mjohansson@equinix.com>
Signed-off-by: Marques Johansson <mjohansson@equinix.com>
Signed-off-by: Marques Johansson <mjohansson@equinix.com>
921c699 to
0b8fe98
Compare
0b8fe98 to
3d0de0f
Compare
|
Rebased to latest I suspect the sweeper shared config may be able to find a better home, sweeper/services/networkedge.go ?) |
3d0de0f to
0a88969
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors Network Edge resources from the equinix package to dedicated packages under internal/resources/networkedge/, addressing part of issue #106. The refactoring reorganizes code structure while consolidating shared utilities like comparisons and nprintf helpers.
Key changes:
- Network Edge resources split into individual packages (device, ssh_key, ssh_user, bgp, acl_template, etc.)
- Resource/DataSource functions renamed from
resourceNetworkX()toResource()(exported) - Shared utilities consolidated (
comparisons.IsEmpty,comparisons.SlicesMatch,nprintf.Nprintf) - Test sweepers moved from test files to dedicated
sweeper.gofiles
Reviewed changes
Copilot reviewed 56 out of 56 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/resources/networkedge/*/resource.go | Refactored resources with package name changes and exported Resource() functions |
| internal/resources/networkedge/*/sweeper.go | New sweeper files extracted from test init() functions |
| internal/resources/networkedge/*/templates_acc_test.go | Test template helpers (duplicated across device, device_link, bgp packages) |
| internal/acceptance/acceptance*.go | Test provider initialization split into separate _test.go file |
| internal/sweep/sweep.go | New SharedConfigForRegion() function for test sweepers |
| internal/comparisons/comparisons.go | Utility functions consolidated from equinix package |
| internal/nprintf/nprintf.go | String formatting helper renamed from NPrintf to Nprintf |
| equinix/network_edge_provider_maps.go | Updated to use new package imports and exported functions |
| equinix/provider.go | Removed deprecated utility functions moved to internal packages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
|
|
||
| func init() { | ||
| TestAccProvider = equinix.Provider() | ||
| TestAccProviders = map[string]*schema.Provider{ | ||
| "equinix": TestAccProvider, | ||
| } | ||
| TestExternalProviders = map[string]resource.ExternalProvider{ | ||
| "random": { | ||
| Source: "hashicorp/random", | ||
| }, | ||
| } | ||
| TestAccFrameworkProvider = provider.CreateFrameworkProvider(version.ProviderVersion).(*provider.FrameworkProvider) |
There was a problem hiding this comment.
The init() function has been moved from internal/acceptance/acceptance.go to this new file. However, the init() function will now run twice if both files are imported - once from acceptance.go and once from acceptance_acc_test.go. Since acceptance_acc_test.go has the _test.go suffix, it's only included in test builds, but acceptance.go is always included. This could cause issues if the package is imported in non-test code. Consider keeping the init() only in acceptance.go or using sync.Once to ensure single initialization.
| ) | |
| func init() { | |
| TestAccProvider = equinix.Provider() | |
| TestAccProviders = map[string]*schema.Provider{ | |
| "equinix": TestAccProvider, | |
| } | |
| TestExternalProviders = map[string]resource.ExternalProvider{ | |
| "random": { | |
| Source: "hashicorp/random", | |
| }, | |
| } | |
| TestAccFrameworkProvider = provider.CreateFrameworkProvider(version.ProviderVersion).(*provider.FrameworkProvider) | |
| "sync" | |
| ) | |
| var initOnce sync.Once | |
| func init() { | |
| initOnce.Do(func() { | |
| TestAccProvider = equinix.Provider() | |
| TestAccProviders = map[string]*schema.Provider{ | |
| "equinix": TestAccProvider, | |
| } | |
| TestExternalProviders = map[string]resource.ExternalProvider{ | |
| "random": { | |
| Source: "hashicorp/random", | |
| }, | |
| } | |
| TestAccFrameworkProvider = provider.CreateFrameworkProvider(version.ProviderVersion).(*provider.FrameworkProvider) | |
| }) |
There was a problem hiding this comment.
Something like this is needed to prevent the problems I initially encountered with this PR, which left it in draft. I believe a better approach was found in other resource sets. ( @ctreatma )
| package device | ||
|
|
||
| import ( | ||
| "github.com/equinix/terraform-provider-equinix/internal/comparisons" | ||
| "github.com/equinix/terraform-provider-equinix/internal/nprintf" | ||
| ) | ||
|
|
||
| const ( | ||
| tstResourcePrefix = "tfacc" | ||
|
|
||
| networkDeviceProjectId = "TF_ACC_NETWORK_DEVICE_PROJECT_ID" | ||
| networkDeviceAccountNameEnvVar = "TF_ACC_NETWORK_DEVICE_BILLING_ACCOUNT_NAME" | ||
| networkDeviceSecondaryAccountNameEnvVar = "TF_ACC_NETWORK_DEVICE_SECONDARY_BILLING_ACCOUNT_NAME" | ||
| networkDeviceMetroEnvVar = "TF_ACC_NETWORK_DEVICE_METRO" | ||
| networkDeviceSecondaryMetroEnvVar = "TF_ACC_NETWORK_DEVICE_SECONDARY_METRO" | ||
| networkDeviceCSRSDWANLicenseFileEnvVar = "TF_ACC_NETWORK_DEVICE_CSRSDWAN_LICENSE_FILE" | ||
| networkDeviceVSRXLicenseFileEnvVar = "TF_ACC_NETWORK_DEVICE_VSRX_LICENSE_FILE" | ||
| networkDeviceVersaController1EnvVar = "TF_ACC_NETWORK_DEVICE_VERSA_CONTROLLER1" | ||
| networkDeviceVersaController2EnvVar = "TF_ACC_NETWORK_DEVICE_VERSA_CONTROLLER2" | ||
| networkDeviceVersaLocalIDEnvVar = "TF_ACC_NETWORK_DEVICE_VERSA_LOCALID" | ||
| networkDeviceVersaRemoteIDEnvVar = "TF_ACC_NETWORK_DEVICE_VERSA_REMOTEID" | ||
| networkDeviceVersaSerialNumberEnvVar = "TF_ACC_NETWORK_DEVICE_VERSA_SERIAL" | ||
| networkDeviceCGENIXLicenseKeyEnvVar = "TF_ACC_NETWORK_DEVICE_CGENIX_LICENSE_KEY" | ||
| networkDeviceCGENIXLicenseSecretEnvVar = "TF_ACC_NETWORK_DEVICE_CGENIX_LICENSE_SECRET" | ||
| networkDevicePANWLicenseTokenEnvVar = "TF_ACC_NETWORK_DEVICE_PANW_LICENSE_TOKEN" | ||
| ) | ||
|
|
||
| type testAccConfig struct { | ||
| ctx map[string]interface{} | ||
| config string | ||
| } | ||
|
|
||
| func newTestAccConfig(ctx map[string]interface{}) *testAccConfig { | ||
| return &testAccConfig{ | ||
| ctx: ctx, | ||
| config: "", | ||
| } | ||
| } | ||
|
|
||
| func (t *testAccConfig) build() string { | ||
| return t.config | ||
| } | ||
|
|
||
| func (t *testAccConfig) withDevice() *testAccConfig { | ||
| t.config += testAccNetworkDevice(t.ctx) | ||
| return t | ||
| } | ||
|
|
||
| func copyMap(source map[string]interface{}) map[string]interface{} { | ||
| target := make(map[string]interface{}) | ||
| for k, v := range source { | ||
| target[k] = v | ||
| } | ||
| return target | ||
| } | ||
|
|
||
| func testAccNetworkDeviceUser(ctx map[string]interface{}) string { | ||
| config := nprintf.Nprintf(` | ||
| resource "equinix_network_ssh_user" "%{user-resourceName}" { | ||
| username = "%{user-username}" | ||
| password = "%{user-password}" | ||
| device_ids = [ | ||
| equinix_network_device.%{device-resourceName}.id`, ctx) | ||
| if _, ok := ctx["device-secondary_name"]; ok { | ||
| config += nprintf.Nprintf(`, | ||
| equinix_network_device.%{device-resourceName}.redundant_id`, ctx) | ||
| } | ||
| config += ` | ||
| ] | ||
| }` | ||
| return config | ||
| } | ||
|
|
||
| func testAccNetworkDevice(ctx map[string]interface{}) string { | ||
| var config string | ||
| config += nprintf.Nprintf(` | ||
| data "equinix_network_account" "test" { | ||
| metro_code = "%{device-metro_code}" | ||
| status = "Active" | ||
| project_id = "%{device-project_id}"`, ctx) | ||
| if v, ok := ctx["device-account_name"]; ok && !comparisons.IsEmpty(v) { | ||
| config += nprintf.Nprintf(` | ||
| name = "%{device-account_name}"`, ctx) | ||
| } | ||
| config += nprintf.Nprintf(` | ||
| }`, ctx) | ||
| if _, ok := ctx["device-secondary_metro_code"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| data "equinix_network_account" "test-secondary" { | ||
| metro_code = "%{device-secondary_metro_code}" | ||
| status = "Active"`, ctx) | ||
| if v, ok := ctx["device-secondary_account_name"]; ok && !comparisons.IsEmpty(v) { | ||
| config += nprintf.Nprintf(` | ||
| name = "%{device-secondary_account_name}"`, ctx) | ||
| } | ||
| config += nprintf.Nprintf(` | ||
| }`, ctx) | ||
| } | ||
| config += nprintf.Nprintf(` | ||
| resource "equinix_network_device" "%{device-resourceName}" { | ||
| self_managed = %{device-self_managed} | ||
| byol = %{device-byol} | ||
| name = "%{device-name}" | ||
| metro_code = "%{device-metro_code}" | ||
| type_code = "%{device-type_code}" | ||
| project_id = "%{device-project_id}" | ||
| package_code = "%{device-package_code}" | ||
| notifications = %{device-notifications} | ||
| term_length = %{device-term_length} | ||
| account_number = data.equinix_network_account.test.number | ||
| version = "%{device-version}" | ||
| core_count = %{device-core_count}`, ctx) | ||
| if _, ok := ctx["device-purchase_order_number"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| purchase_order_number = "%{device-purchase_order_number}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-purchase_order_number"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| order_reference = "%{device-order_reference}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-additional_bandwidth"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| additional_bandwidth = %{device-additional_bandwidth}`, ctx) | ||
| } | ||
| if _, ok := ctx["device-throughput"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| throughput = %{device-throughput} | ||
| throughput_unit = "%{device-throughput_unit}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-hostname"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| hostname = "%{device-hostname}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-interface_count"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| interface_count = %{device-interface_count}`, ctx) | ||
| } | ||
| if _, ok := ctx["acl-resourceName"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| acl_template_id = equinix_network_acl_template.%{acl-resourceName}.id`, ctx) | ||
| } | ||
| if _, ok := ctx["mgmtAcl-resourceName"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| mgmt_acl_template_uuid = equinix_network_acl_template.%{mgmtAcl-resourceName}.id`, ctx) | ||
| } | ||
| if _, ok := ctx["sshkey-resourceName"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| ssh_key { | ||
| username = "test" | ||
| key_name = equinix_network_ssh_key.%{sshkey-resourceName}.name | ||
| }`, ctx) | ||
| } | ||
| if _, ok := ctx["device-license_file"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| license_file = "%{device-license_file}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-vendorConfig_enabled"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| vendor_configuration = {`, ctx) | ||
| if _, ok := ctx["device-vendorConfig_siteId"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| siteId = "%{device-vendorConfig_siteId}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-vendorConfig_systemIpAddress"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| systemIpAddress = "%{device-vendorConfig_systemIpAddress}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-vendorConfig_licenseKey"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| licenseKey = "%{device-vendorConfig_licenseKey}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-vendorConfig_licenseSecret"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| licenseSecret = "%{device-vendorConfig_licenseSecret}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-vendorConfig_controller1"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| controller1 = "%{device-vendorConfig_controller1}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-vendorConfig_controller2"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| controller2 = "%{device-vendorConfig_controller2}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-vendorConfig_localId"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| localId = "%{device-vendorConfig_localId}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-vendorConfig_remoteId"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| remoteId = "%{device-vendorConfig_remoteId}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-vendorConfig_serialNumber"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| serialNumber = "%{device-vendorConfig_serialNumber}"`, ctx) | ||
| } | ||
| config += nprintf.Nprintf(` | ||
| }`, ctx) | ||
| } | ||
| if _, ok := ctx["device-secondary_name"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| secondary_device { | ||
| name = "%{device-secondary_name}"`, ctx) | ||
| if _, ok := ctx["device-secondary_metro_code"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| metro_code = "%{device-secondary_metro_code}" | ||
| account_number = data.equinix_network_account.test-secondary.number`, ctx) | ||
| } else { | ||
| config += nprintf.Nprintf(` | ||
| metro_code = "%{device-metro_code}" | ||
| account_number = data.equinix_network_account.test.number`, ctx) | ||
| } | ||
| config += nprintf.Nprintf(` | ||
| notifications = %{device-secondary_notifications}`, ctx) | ||
| if _, ok := ctx["device-secondary_additional_bandwidth"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| additional_bandwidth = %{device-secondary_additional_bandwidth}`, ctx) | ||
| } | ||
| if _, ok := ctx["device-secondary_hostname"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| hostname = "%{device-secondary_hostname}"`, ctx) | ||
| } | ||
| if _, ok := ctx["acl-secondary_resourceName"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| acl_template_id = equinix_network_acl_template.%{acl-secondary_resourceName}.id`, ctx) | ||
| } | ||
| if _, ok := ctx["mgmtAcl-secondary_resourceName"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| mgmt_acl_template_uuid = equinix_network_acl_template.%{mgmtAcl-secondary_resourceName}.id`, ctx) | ||
| } | ||
| if _, ok := ctx["sshkey-resourceName"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| ssh_key { | ||
| username = "test" | ||
| key_name = equinix_network_ssh_key.%{sshkey-resourceName}.name | ||
| }`, ctx) | ||
| } | ||
| if _, ok := ctx["device-secondary_license_file"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| license_file = "%{device-secondary_license_file}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-secondary_vendorConfig_enabled"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| vendor_configuration = {`, ctx) | ||
| if _, ok := ctx["device-secondary_vendorConfig_siteId"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| siteId = "%{device-secondary_vendorConfig_siteId}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-secondary_vendorConfig_systemIpAddress"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| systemIpAddress = "%{device-secondary_vendorConfig_systemIpAddress}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-secondary_vendorConfig_licenseKey"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| licenseKey = "%{device-secondary_vendorConfig_licenseKey}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-secondary_vendorConfig_licenseSecret"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| licenseSecret = "%{device-secondary_vendorConfig_licenseSecret}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-secondary_vendorConfig_controller1"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| controller1 = "%{device-secondary_vendorConfig_controller1}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-secondary_vendorConfig_controller2"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| controller2 = "%{device-secondary_vendorConfig_controller2}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-secondary_vendorConfig_localId"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| localId = "%{device-secondary_vendorConfig_localId}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-secondary_vendorConfig_remoteId"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| remoteId = "%{device-secondary_vendorConfig_remoteId}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-secondary_vendorConfig_serialNumber"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| serialNumber = "%{device-secondary_vendorConfig_serialNumber}"`, ctx) | ||
| } | ||
| config += nprintf.Nprintf(` | ||
| }`, ctx) | ||
| } | ||
| config += ` | ||
| }` | ||
| } | ||
| if _, ok := ctx["device-cluster_name"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| cluster_details { | ||
| cluster_name = "%{device-cluster_name}"`, ctx) | ||
| config += ` | ||
| node0 {` | ||
| if _, ok := ctx["device-node0_license_file_id"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| license_file_id = "%{device-node0_license_file_id}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-node0_license_token"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| license_token = "%{device-node0_license_token}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-node0_vendorConfig_enabled"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| vendor_configuration {`, ctx) | ||
| if _, ok := ctx["device-node0_vendorConfig_hostname"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| hostname = "%{device-node0_vendorConfig_hostname}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-node0_vendorConfig_adminPassword"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| admin_password = "%{device-node0_vendorConfig_adminPassword}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-node0_vendorConfig_controller1"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| controller1 = "%{device-node0_vendorConfig_controller1}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-node0_vendorConfig_activationKey"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| activation_key = "%{device-node0_vendorConfig_activationKey}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-node0_vendorConfig_controllerFqdn"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| controller_fqdn = "%{device-node0_vendorConfig_controllerFqdn}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-node0_vendorConfig_rootPassword"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| root_password = "%{device-node0_vendorConfig_rootPassword}"`, ctx) | ||
| } | ||
| config += nprintf.Nprintf(` | ||
| }`, ctx) | ||
| } | ||
| config += ` | ||
| }` | ||
| config += ` | ||
| node1 {` | ||
| if _, ok := ctx["device-node1_license_file_id"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| license_file_id = "%{device-node1_license_file_id}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-node1_license_token"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| license_token = "%{device-node1_license_token}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-node1_vendorConfig_enabled"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| vendor_configuration {`, ctx) | ||
| if _, ok := ctx["device-node1_vendorConfig_hostname"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| hostname = "%{device-node1_vendorConfig_hostname}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-node1_vendorConfig_adminPassword"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| admin_password = "%{device-node1_vendorConfig_adminPassword}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-node1_vendorConfig_controller1"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| controller1 = "%{device-node1_vendorConfig_controller1}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-node1_vendorConfig_activationKey"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| activation_key = "%{device-node1_vendorConfig_activationKey}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-node1_vendorConfig_controllerFqdn"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| controller_fqdn = "%{device-node1_vendorConfig_controllerFqdn}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-node1_vendorConfig_rootPassword"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| root_password = "%{device-node1_vendorConfig_rootPassword}"`, ctx) | ||
| } | ||
| config += nprintf.Nprintf(` | ||
| }`, ctx) | ||
| } | ||
| config += ` | ||
| }` | ||
| config += ` | ||
| }` | ||
| } | ||
| config += ` | ||
| }` | ||
| return config | ||
| } | ||
|
|
||
| func testAccNetworkDeviceACL(ctx map[string]interface{}) string { | ||
| var config string | ||
| if _, ok := ctx["acl-name"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| resource "equinix_network_acl_template" "%{acl-resourceName}" { | ||
| name = "%{acl-name}" | ||
| description = "%{acl-description}" | ||
| inbound_rule { | ||
| subnet = "10.0.0.0/24" | ||
| protocol = "IP" | ||
| src_port = "any" | ||
| dst_port = "any" | ||
| } | ||
| }`, ctx) | ||
| } | ||
| if _, ok := ctx["mgmtAcl-name"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| resource "equinix_network_acl_template" "%{mgmtAcl-resourceName}" { | ||
| name = "%{mgmtAcl-name}" | ||
| description = "%{mgmtAcl-description}" | ||
| inbound_rule { | ||
| subnet = "11.0.0.0/24" | ||
| protocol = "IP" | ||
| src_port = "any" | ||
| dst_port = "any" | ||
| } | ||
| }`, ctx) | ||
| } | ||
| if _, ok := ctx["acl-secondary_name"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| resource "equinix_network_acl_template" "%{acl-secondary_resourceName}" { | ||
| name = "%{acl-secondary_name}" | ||
| description = "%{acl-secondary_description}" | ||
| inbound_rule { | ||
| subnet = "192.0.0.0/24" | ||
| protocol = "IP" | ||
| src_port = "any" | ||
| dst_port = "any" | ||
| } | ||
| }`, ctx) | ||
| } | ||
| if _, ok := ctx["mgmtAcl-secondary_name"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| resource "equinix_network_acl_template" "%{mgmtAcl-secondary_resourceName}" { | ||
| name = "%{mgmtAcl-secondary_name}" | ||
| description = "%{mgmtAcl-secondary_description}" | ||
| inbound_rule { | ||
| subnet = "193.0.0.0/24" | ||
| protocol = "IP" | ||
| src_port = "any" | ||
| dst_port = "any" | ||
| } | ||
| }`, ctx) | ||
| } | ||
| return config | ||
| } | ||
|
|
||
| func testAccNetworkDeviceSSHKey(ctx map[string]interface{}) string { | ||
| return nprintf.Nprintf(` | ||
| resource "equinix_network_ssh_key" "%{sshkey-resourceName}" { | ||
| name = "%{sshkey-name}" | ||
| public_key = "%{sshkey-public_key}" | ||
| } | ||
| `, ctx) | ||
| } | ||
|
|
||
| func (t *testAccConfig) withACL() *testAccConfig { | ||
| t.config += testAccNetworkDeviceACL(t.ctx) | ||
| return t | ||
| } | ||
|
|
||
| func (t *testAccConfig) withSSHKey() *testAccConfig { | ||
| t.config += testAccNetworkDeviceSSHKey(t.ctx) | ||
| return t | ||
| } | ||
|
|
||
| func (t *testAccConfig) withSSHUser() *testAccConfig { | ||
| t.config += testAccNetworkDeviceUser(t.ctx) | ||
| return t | ||
| } |
There was a problem hiding this comment.
The same test template code (testAccNetworkDevice, testAccNetworkDeviceACL, testAccNetworkDeviceSSHKey, copyMap, testAccNetworkDeviceUser, newTestAccConfig, testAccConfig struct, and related methods) is duplicated across three files:
internal/resources/networkedge/device/templates_acc_test.go(lines 1-461)internal/resources/networkedge/device_link/templates_acc_test.go(lines 1-461)internal/resources/networkedge/bgp/templates_acc_test.go(lines 1-461)
This violates the DRY principle and creates a maintenance burden. Consider:
- Moving this shared test infrastructure to a common package like
internal/resources/networkedge/testutilsorinternal/acceptance - Or creating a shared
templates_test_common.gofile that all three packages can import
| if _, ok := ctx["device-purchase_order_number"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| order_reference = "%{device-order_reference}"`, ctx) |
There was a problem hiding this comment.
The condition on line 117 checks for device-purchase_order_number but should likely check for device-order_reference instead. This is a copy-paste error that causes order_reference to only be added when purchase_order_number exists in the context, which may not be the intended behavior.
| if _, ok := ctx["device-purchase_order_number"]; ok { | ||
| config += nprintf.Nprintf(` | ||
| order_reference = "%{device-order_reference}"`, ctx) |
There was a problem hiding this comment.
The condition on line 117 checks for device-purchase_order_number but should likely check for device-order_reference instead. This is a copy-paste error that causes order_reference to only be added when purchase_order_number exists in the context, which may not be the intended behavior.
| config += nprintf.Nprintf(` | ||
| purchase_order_number = "%{device-purchase_order_number}"`, ctx) | ||
| } | ||
| if _, ok := ctx["device-purchase_order_number"]; ok { |
There was a problem hiding this comment.
The condition on line 117 checks for device-purchase_order_number but should likely check for device-order_reference instead. This is a copy-paste error that causes order_reference to only be added when purchase_order_number exists in the context, which may not be the intended behavior.
| if _, ok := ctx["device-purchase_order_number"]; ok { | |
| if _, ok := ctx["device-order_reference"]; ok { |
Signed-off-by: Marques Johansson <mjohansson@equinix.com>
Signed-off-by: Marques Johansson <mjohansson@equinix.com>
Signed-off-by: Marques Johansson <mjohansson@equinix.com>
Signed-off-by: Marques Johansson <mjohansson@equinix.com>
…ovider-equinix on networkedge files Signed-off-by: Marques Johansson <mjohansson@equinix.com>
0a88969 to
3b46f1d
Compare
This PR refactors Network Edge resources from the
equinixpackage to dedicated packages underinternal/resources/networkedge/, addressing part of issue #106. The refactoring reorganizes code structure while consolidating shared utilities likecomparisonsandnprintfhelpers.Key changes:
device,ssh_key,ssh_user,bgp,acl_template, etc.)resourceNetworkX()toResource()(exported)sweeper.gofilescomparisons.IsEmpty,comparisons.SlicesMatch,nprintf.Nprintf)Refactoring and modularization of shared utilities:
stringsFound,isEmpty,slicesMatch) fromequinix/provider.goand their tests fromequinix/provider_test.gointo the internal packageinternal/comparisons, removing deprecated code from the provider and test files. [1] [2]nprintfhelper function into a new internal packageinternal/nprintf, updating documentation and function naming for clarity. [1] [2] [3]Test and sweeper code cleanup:
resource_network_ssh_user_acc_test.gofile, consolidating SSH user acceptance tests and sweepers, as these are now covered elsewhere or are redundant due to the new structure.equinix/equinix_sweeper_test.goandequinix/provider_test.go. [1] [2] [3]Provider and resource mapping improvements:
network_edge_provider_maps.goto use resources and data sources from their respective internal packages, improving modularity and separation of concerns.Acceptance test initialization refactor:
internal/acceptance/acceptance.goto a new fileinternal/acceptance/acceptance_acc_test.go, streamlining test setup and clarifying responsibilities. [1] [2] [3]General codebase hygiene:
These changes collectively improve the structure and maintainability of the codebase, making shared logic easier to test and reuse, and laying the groundwork for further modularization and package separation.