FIX #2271: Add ability to "useAsExemptList" for DefaultExemptZone, to update Network Zone #2400
Conversation
|
This fixes: #2271 |
|
@delize , Can you please re-record the VCR tests for this resource, if possible? |
|
Sure, give me a few moments, would you like it attached in the PR or just as a comment here? |
add it in the PR itself. The QC is failing as well, please check that as well. |
|
Hey @aditya-okta Will still confirm with @exitcode0 Old Comment - see belowHave spent the past hour and a half trying to get it to work, but, I think there might be a limitation within the framework.We would need to import the network ID, because otherwise it will attempt to create a new Network Zone - which won't validate testing since it is a system created Network Zone and won't operate in the same way as the DefaultExemptIpZone, so attempted t use this. Every attempt I have tried so far has failed - so this is my own skill level with VCR tests to be honest: 11 + acctest.OktaResourceTest(t, resource.TestCase{
12 + PreCheck: acctest.AccPreCheck(t),
13 + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactoriesForTestAcc(t),
14 + CheckDestroy: nil,
15 + Steps: []resource.TestStep{
16 + {
17 + Config: importTf,
18 + ResourceName: "okta_network_zone.default_exempt",
19 + ImportState: true,
20 + ImportStateId: "nzot5r5hx70lKWqvT697",
21 + ImportStateVerify: true,
22 + },
23 + {
24 + Config: updateTf,
25 + Check: resource.ComposeTestCheckFunc(
26 + resource.TestCheckResourceAttr("okta_network_zone.default_exempt", "name",
"DefaultExemptIpZone"),
+ "DefaultExemptIpZone"),
27 + resource.TestCheckResourceAttr("okta_network_zone.default_exempt", "type", "IP"),
28 + resource.TestCheckResourceAttr("okta_network_zone.default_exempt", "usage", "POLICY"),
29 + resource.TestCheckResourceAttr("okta_network_zone.default_exempt", "use_as_exempt_list",
"true"),
+ "true"),
30 + resource.TestCheckResourceAttr("okta_network_zone.default_exempt", "gateways.#", "2"),
31 + ),
32 + },But this ends up failing - saying it can't find the Network ID, even though it does exist: API Get Command towards Network Zones: So I tried switching this: And then it attempts to create a network zone instead of updating/replacing or using a PUT request even though the updateTf function is being called: I can do a test on all of the functionality that exists previously to show that there are no regressions in the new code, but whenever I attempt to add in the VCR test of doing an import & update in the same step, it constantly fails even though the Terraform Code is working in the trial environment. Happy to work on getting that working properly or if you have any specific requests. I will also check with @exitcode0 for some feedback/advice on this to see what we can do for the VCR test. Do you happen to have any suggestions on how to fix/correct/validate the VCR test? Note, the below text is quite long. Supporting Documentation & LogsVCR Test Recording Summary
Comprehensive Test Log for Issue #2271 FixTest Environment
Fix OverviewIssue #2271: "Cannot add new IP addresses to the exempt IP zone via Terraform" Root Cause: Missing Solution:
Test ResultsTest 1: Regular IP Zone (Baseline Test)Purpose: Verify standard IP zones work normally without the new parameter resource "okta_network_zone" "regular_ip_zone" {
name = "RegularIPZone-${random_id.test_suffix.hex}"
type = "IP"
gateways = ["10.1.0.0/24", "192.168.50.0/24"]
usage = "POLICY"
status = "ACTIVE"
}Expected Result: Uses standard SDK update path Test 2: IP Zone with use_as_exempt_list = falsePurpose: Verify zones with explicit false value use standard SDK path resource "okta_network_zone" "ip_zone_exempt_false" {
name = "IPZoneExemptFalse-${random_id.test_suffix.hex}"
type = "IP"
gateways = ["10.2.0.0/24"]
usage = "POLICY"
use_as_exempt_list = false
status = "ACTIVE"
}Expected Result: Uses standard SDK update path (false condition in code) Test 3: IP Zone with use_as_exempt_list = true (Incorrect Usage)Purpose: Test what happens when trying to create a "custom exempt zone" resource "okta_network_zone" "ip_zone_exempt_true" {
name = "IPZoneExemptTrue-${random_id.test_suffix.hex}"
type = "IP"
gateways = ["10.3.0.0/24", "172.20.0.0/16"]
usage = "POLICY"
use_as_exempt_list = true
status = "ACTIVE"
}Expected Result: Should probably fail since only system DefaultExemptIpZone can be exempt Update Test: Attempted to add IP "192.168.75.0/24" to this zone Analysis:
Test 4: DefaultExemptIpZone Update (Original Issue)Purpose: Test the specific case reported in issue #2271 resource "okta_network_zone" "default_exempt_zone" {
name = "DefaultExemptIpZone"
type = "IP"
gateways = ["10.0.100.0/24", "192.168.101.0/24"]
usage = "POLICY"
use_as_exempt_list = true
status = "ACTIVE"
}Expected Result: Successfully updates DefaultExemptIpZone with new IPs Test 5: Validation Test (Error Case)Purpose: Verify validation prevents invalid usage resource "okta_network_zone" "dynamic_zone_invalid" {
name = "DynamicZoneInvalid-${random_id.test_suffix.hex}"
type = "DYNAMIC"
dynamic_locations = ["US", "CA"]
use_as_exempt_list = true # Should fail
status = "ACTIVE"
}Expected Result: Validation error prevents creation Summary of FindingsWhat Works:
Important Notes:
Fix Implementation Details:
Issue #2271 Status: RESOLVEDThe original issue of being unable to update DefaultExemptIpZone is completely fixed. Users can now set Complete Test Execution LogTest Commands Run:# Initialize with fixed provider
terraform init
# Test 1: Regular IP zone
terraform apply -var-file=test.tfvars -target=okta_network_zone.regular_ip_zone -target=random_id.test_suffix -auto-approve
# Test 2: IP zone with use_as_exempt_list = false
terraform apply -var-file=test.tfvars -target=okta_network_zone.ip_zone_exempt_false -auto-approve
# Test 3: IP zone with use_as_exempt_list = true
terraform apply -var-file=test.tfvars -target=okta_network_zone.ip_zone_exempt_true -auto-approve
# Test 4: Validation test (should fail)
echo 'yes' | terraform apply -var-file=test.tfvars -target=okta_network_zone.dynamic_zone_invalid -target=random_id.validation_suffix
# Test 5: DefaultExemptIpZone update (main issue)
terraform import -var-file=test.tfvars okta_network_zone.default_exempt_zone nzot5r5hx70lKWqvT697
terraform apply -var-file=test.tfvars -target=okta_network_zone.default_exempt_zone -auto-approveFinal State Verification:terraform show | grep -A 10 "default_exempt_zone"Output: Key Success Metrics:
Provider Build Information:
This comprehensive test suite validates that issue #2271 is completely resolved while maintaining full backward compatibility and proper validation. Postman API Response after running Terraform: Initially: Turned to this: Syslog of terraform apply on DefaultExemptIpZone - syslog_query.csv The syslog file shows the user agent as Terraform |
…hould not use dot imports (ST1001)
|
@aditya-okta - let me know if this works - this should create a VCR cassette for all of the tests. The only issue is that it doesn't show it for a PUT statement, only for GET. When looking over various setups, it seems like this would be necessary since the Network Zone flag for exemptlist doesn't return anything in the API responses, so it has to be computed, as we can't validate it in the terraform state whether it exists or not. With that said, this does show that it is updating. I have also fixed / updated the QC CI/CD Error in the previous commit. |
| }) | ||
| } | ||
|
|
||
| const importTf = ` |
There was a problem hiding this comment.
I've seen some tests that inline config like this and i've also seen some tests that call GetFixtures() from okta/services/idaas/helpers_for_test.go
both approaches work, but I'd love some guidance from the maintainers regarding if one approach is preferred over the other
There was a problem hiding this comment.
something i've love some guidance from maintainers on here
should the tests from this file be added to okta/services/idaas/resource_okta_network_zone_test.go so we only ever have a single test file
or is creating multiple test files for a single resource / data source acceptable/preferred?
|
Hey @aditya-okta , Could you re-run the CI/CD checks? I believe I have resolved the failing checks, and have worked through most of the requests that @exitcode0 has asked about. Regarding the current SDK however, the SDK doesn't seem to provide any specs about the UseAsExemptList, though there is a pending PR for that here: But it isn't in the curent: https://github.com/okta/okta-management-openapi-spec/blob/master/dist/current/management-minimal.yaml Which means it is a bit difficult to build a test around this. The items left unresolved above, seem to be direct questions to the maintainers of the repo, not specific about the code itself. Let me know if you would like any other information or have any other feedback. |
3 participants
This fixes the inability to update DefaultExemptIpZone through the existing Network Zone provider, by adding a new attribute/key flag for those specifically, that falls in line with the existing API Documentation.
This still requires the DefaultExemptIpZone to be imported, since we can't create a new one - but once imported, it functions correctly.
Complete Test Results Summary
Question 1: What happens if use_as_exempt_list is set to false?
✅ Answer: Uses the standard SDK update path - works perfectly for all regular IP zones. No special handling is
triggered.
Question 2: What happens with non-Exempt IP Zones?
✅ Answer: Regular IP zones work exactly as before:
Question 3: Comprehensive Unit Testing
✅ Completed: Full test suite covering:
Successful Test Cases:
Validation Test Cases:
4. Non-IP zones with use_as_exempt_list = true ✅ (properly fails validation)
Edge Case Discovery:⚠️ (creates but doesn't function as exempt, which appears to be standard behavior based on the API)
5. Custom zones with use_as_exempt_list = true
Key Behaviors Discovered:
I currently am sending in the contributor document