Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add ibm_cbr_zone_addresses resource #5505

Merged
merged 12 commits into from
Jul 18, 2024
Merged

Add ibm_cbr_zone_addresses resource #5505

merged 12 commits into from
Jul 18, 2024

Conversation

mtbrandy
Copy link
Contributor

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #0000

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccIBMCbr'

=== RUN   TestAccIBMCbrRuleDataSourceBasic
--- PASS: TestAccIBMCbrRuleDataSourceBasic (12.50s)
=== RUN   TestAccIBMCbrRuleDataSourceAllArgs
--- PASS: TestAccIBMCbrRuleDataSourceAllArgs (11.93s)
=== RUN   TestAccIBMCbrZoneAddressesDataSourceBasic
--- PASS: TestAccIBMCbrZoneAddressesDataSourceBasic (10.37s)
=== RUN   TestAccIBMCbrZoneAddressesDataSourceMultiple
--- PASS: TestAccIBMCbrZoneAddressesDataSourceMultiple (11.08s)
=== RUN   TestAccIBMCbrZoneDataSourceBasic
--- PASS: TestAccIBMCbrZoneDataSourceBasic (10.58s)
=== RUN   TestAccIBMCbrZoneDataSourceAllArgs
--- PASS: TestAccIBMCbrZoneDataSourceAllArgs (11.10s)
=== RUN   TestAccIBMCbrRuleBasic
--- PASS: TestAccIBMCbrRuleBasic (10.72s)
=== RUN   TestAccIBMCbrRuleAllArgs
--- PASS: TestAccIBMCbrRuleAllArgs (23.31s)
=== RUN   TestAccIBMCbrZoneAddressesBasic
--- PASS: TestAccIBMCbrZoneAddressesBasic (26.35s)
=== RUN   TestAccIBMCbrZoneAddressesUpdate
--- PASS: TestAccIBMCbrZoneAddressesUpdate (38.00s)
=== RUN   TestAccIBMCbrZoneBasic
--- PASS: TestAccIBMCbrZoneBasic (9.97s)
=== RUN   TestAccIBMCbrZoneAllArgs
--- PASS: TestAccIBMCbrZoneAllArgs (19.64s)
PASS

@mtbrandy
Copy link
Contributor Author

mtbrandy commented Jul 12, 2024 •
edited
Loading

This PR requires github.com/IBM/platform-services-go-sdk version v0.64.4 or later.

There is a compatibility issue with that version of the SDK that is causing the current build failure. #5499 fixes that issue and, thus, that PR must be merged before this one.

@mtbrandy mtbrandy marked this pull request as ready for review July 15, 2024 13:38
@mtbrandy
Copy link
Contributor Author

@hkantare This PR is now ready for review now that #5499 has been merged.

@zhenwan
Copy link
Contributor

zhenwan commented Jul 15, 2024

please make sure you do this manual test:

  • create address resource from terraform
  • delete the zone from outside of terraform, like CLI/UI/API
  • run terraform plan / apply

@zhenwan
Copy link
Contributor

zhenwan commented Jul 15, 2024

reviewed the PR, sent some comments directly to Matt

@mtbrandy
Copy link
Contributor Author

please make sure you do this manual test:

  • create address resource from terraform
  • delete the zone from outside of terraform, like CLI/UI/API
  • run terraform plan / apply

Thanks, I have pushed a commit to fix an issue found during this test.

hkantare
hkantare reviewed Jul 17, 2024
View reviewed changes
ibm/service/contextbasedrestrictions/resource_ibm_cbr_zone_addresses.go Outdated

func resourceIBMCbrZoneAddressesCreate(context context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
addressId, _ := uuid.GenerateUUID()
d.SetId(composeZoneAddressesId(d.Get("zone_id").(string), "TF-"+addressId))
Copy link
Collaborator

Choose a reason for hiding this comment

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

We are setting ID before we actually add addresses in resourceReplaceZoneAddresses method which is in correct .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in latest commit.

ibm/service/contextbasedrestrictions/resource_ibm_cbr_zone_addresses.go Outdated
if err != nil {
return diag.FromErr(fmt.Errorf("Error setting addresses: %s", err))
}
err = resourceReplaceZoneAddresses(context, d, meta, true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does resourceReplaceZoneAddresses do when its destroy.

Can we just do d.SetId("") and return if we don't have Delete specific API

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resourceReplaceZoneAddresses is called to remove any associated addresses from the zone before deleting the zone addresses TF resource.

@hkantare
Copy link
Collaborator

Fix the conflicts in go.mod

@hkantare hkantare merged commit c12b514 into IBM-Cloud:master Jul 18, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hkantare hkantare hkantare left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mtbrandy @zhenwan @hkantare