Add PhysicalAddress and CapabilitiesManagedBySatellite to Terraform SatelliteLocation for both resource and data-source #5530

FishAndFrog · 2024-07-25T15:22:41Z

PhysicalAddress and CapabilitiesManagedBySatellite have been added as new parameters to SatelliteLocationAPIs already. Here, Address has been added to Create and Get APIs, whereas Capabilities has been added to Create, Get and List APIs.
I would like to submit this Pull-Request, in order to extend Address and Capabilities in the resource and data-source methods of Terraform-Provider-IBM for Satellite-Location Create and Get.

See what makes a good Pull Request at : https://github.com/IBM-Cloud/terraform-provider-ibm/blob/master/.github/CONTRIBUTING.md

Output from acceptance testing:

Resource Test:
In the following test output, some test fail due to coreos_enabled and post-test destroy issues. But the test TestAccSatelliteLocation_PodAndServiceSubnet does pass and does contain the two new parameter of Satellite-Location (physical_address and capabilities) for test. Thus, the two coreos_enabled and post-test destroy issues can be solved post merging the changes from this Pull Request.

https://ibm-argonauts.slack.com/archives/GDAE57ZPA/p1721633692291339?thread_ts=1721042399.405719&cid=GDAE57ZPA

Data-Source Test:
The data-source tests for physical-address and capabilities do pass:

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

…ddress and Capabilities

…rraform with Address and Capabilities

…ddress and Capabilities

…AndFrog/terraform-provider-ibm into add_satloc_address_and_capabilities

Add Capabilities tests for Create and Get Satellite Location.

Add capabilities to the tests in satloc_nlb_dns

…ervices-go-sdk@454a2ae

Add SDK Version, documentation and others

…n_test.go

FishAndFrog · 2024-07-25T15:31:59Z

Hi Zoltan @TwoDCube, hi Harini @hkantare, kindly requesting you for a review and merge respectively. Many thanks to both of you in advance : )

hkantare · 2024-07-30T09:53:57Z

ibm/service/satellite/data_source_ibm_satellite_location.go

@@ -31,6 +32,18 @@ func DataSourceIBMSatelliteLocation() *schema.Resource {
 				Computed:    true,
 				Description: "The IBM Cloud metro from which the Satellite location is managed",
 			},
+			"physical_address": {
+				Type:        schema.TypeString,
+				Optional:    true,


In datasource physical_address is computed n't optional where user can provide some input

Thanks. Done

hkantare · 2024-07-30T10:01:23Z

website/docs/r/satellite_location.html.markdown

@@ -85,6 +87,8 @@ Review the argument references that you can specify for your resource.
  Nested scheme for `cos_credentials`:
  - `access_key-id` - (Required, String)The `HMAC` secret access key ID.
  - `secret_access_key`-  (Optional, String) The `HMAC` secret access key.
+- `physical_address` -  (Optional, String) The physical address of the new Satellite location which is deployed on premise.
+- `capabilities` - (Optional, Array of Strings) Satellite capabilities attached to the location. It is mandatory to add the value 'on-prem' to 'capabilitiesManagedBySatellite', if a value has been set for 'physicalAddress'. On the other hand, value can be added to 'capabilitiesManagedBySatellite' without setting any value to 'physicalAddress'. In other words, 'capabilitiesManagedBySatellite' is optional, unless 'physicalAddress' gets set.


This description is n't clear what do you mean capabilitiesManagedBySatellite here
Can you check if you can add any RequiredWith at schema level

This is because, in Kubernetes API, capabilities is called capabilitiesManagedBySatellite. Now, I also added the API link with this info to the markdown.
Thanks for the RequiredWith tipp. Also, added RequiredWith-physical_address to capabilities in resource code.

ibm/flex/structures.go

TwoDCube · 2024-07-30T09:55:21Z

ibm/service/satellite/data_source_ibm_satellite_location.go

@@ -31,6 +32,18 @@ func DataSourceIBMSatelliteLocation() *schema.Resource {
 				Computed:    true,
 				Description: "The IBM Cloud metro from which the Satellite location is managed",
 			},
+			"physical_address": {
+				Type:        schema.TypeString,
+				Optional:    true,


This should be

Suggested change

Optional: true,

Computed: true,

This is a data source and the physical address is not helping to retrieve the resource from the provider. This is meant to be a field that is "read" from the API.

Thanks. Done

TwoDCube · 2024-07-30T09:56:29Z

ibm/service/satellite/data_source_ibm_satellite_location_nlb_dns_test.go

I think it would be better if we just not set the physical address and capabilities here, there is no need to test this change in every unrelated test

TwoDCube · 2024-07-30T10:02:42Z

ibm/service/satellite/resource_ibm_satellite_location.go

+			"physical_address": {
+				Type:        schema.TypeString,
+				Optional:    true,
+				Description: "An optional physical address of the new Satellite location which is deployed on premise",
+			},
+			"capabilities": {
+				Type:        schema.TypeSet,
+				Optional:    true,
+				Elem:        &schema.Schema{Type: schema.TypeString},
+				Set:         schema.HashString,
+				Description: "The satellite capabilities attached to the location",
+			},


I'm not familiar with the API, but in case these can be only set once during create time and not updated we need DiffSuppressFunc: flex.ApplyOnce

Yes, you are right that physical_address and capabilites cannot be changed after creation.

Thanks. Done

…ties

…ilities

FishAndFrog

Hi Zoltan @TwoDCube, hi Harini @hkantare, your review findings have been now updated. Requesting you to please have a look once again. Thanks in advance

Shreenkhala-Keller and others added 30 commits May 13, 2024 09:55

Changes to add Address

f4065ba

Merge branch 'IBM-Cloud:master' into add_address

875bd8d

Changes to add Address in data_source_ibm_satellite_location.go

8e658cb

Merge branch 'add_address' into new_add_address

03367c1

Merge branch 'IBM-Cloud:master' into new_add_address

81f2a7e

Add Address and Capabilities to Create-Satellite-Location

ad759ad

Add test for Address to Create-Satellite-Location tests

82172bf

Add Address and Capabilities to Get-Satellite-Location

1eb0133

Add Address Tests to Get-Satellite-Location tests

7488b92

Add new List-Satellite-Location functionality in ibm-terraform with A…

4860cdc

…ddress and Capabilities

Add tests for the new List-Satellite-Location functionality in ibm-te…

171eb2d

…rraform with Address and Capabilities

Add new List-Satellite-Location functionality in ibm-terraform with A…

ae0ffda

…ddress and Capabilities

Merge branch 'IBM-Cloud:master' into add_satloc_address_and_capabilities

31c5a64

Replace container-SDK with own fork changes

b144a93

Merge branch 'IBM-Cloud:master' into add_satloc_address_and_capabilities

9b64dca

Replaced Capabilities by CapabilitiesManagedBySatellite

3a08f97

Replaced Capabilities by CapabilitiesManagedBySatellite

91fad75

fix compile errors

08f3554

Merge branch 'add_satloc_address_and_capabilities' of github.com:Fish…

b48717e

…AndFrog/terraform-provider-ibm into add_satloc_address_and_capabilities

Merge branch 'IBM-Cloud:master' into master

5e3c134

Comment removal

214b837

Add capabilities tests

8423809

Add test for capabilities to create-satellite-location

6907245

Add test for capabilities for get data of satellite-location

9398f10

Merge pull request #3 from FishAndFrog/capabilities_tests_jul9

8cc311d

Add Capabilities tests for Create and Get Satellite Location.

Add capabilities to the tests in satloc_nlb_dns

5752a6c

Merge pull request #4 from FishAndFrog/capabilities_tests_jul9

47e36bd

Add capabilities to the tests in satloc_nlb_dns

Merge branch 'IBM-Cloud:master' into master

1d31881

Replace IBM-Cloud/container-services-go-sdk wit IBM-Cloud/container-s…

abc8869

…ervices-go-sdk@454a2ae

Correct on-prem string

c6a0bb2

Shreenkhala-Keller and others added 7 commits July 25, 2024 10:43

Add test-place in test

28ccb51

Add test-place in test

4f89308

Add documentation for PhysicalAddress and Capabilities for Resource

610ed0b

Add documentation for PhysicalAddress and Capabilities for Data Source

0a15585

Merge pull request #5 from FishAndFrog/new_addr_cap_doku

c6b5971

Add SDK Version, documentation and others

Correct format for capabilities

687d338

Correct the format for capabilities in resource_ibm_satellite_locatio…

87fc7c0

…n_test.go

Shreenkhala-Keller added 3 commits July 26, 2024 08:03

Add capabilities test in resource_ibm_satellite_location_test.go

8f5a9be

Add capabilities test in data_source_ibm_satellite_location_test.go

1c0fbbc

Corrected indentation

1f79391

hkantare reviewed Jul 30, 2024

View reviewed changes

TwoDCube reviewed Jul 30, 2024

View reviewed changes

Shreenkhala-Keller added 7 commits July 30, 2024 15:47

Change FlattenSatelliteWorkerCapabilities to FlattenSatelliteCapabili…

19c3094

…ties

Change FlattenSatelliteWorkerCapabilities to FlattenSatelliteCapabili…

606c8b3

…ties

Replace Optional with Computed in data_source_ibm_satellite_location.go

7ed3cd1

Add 'DiffSuppressFunc: flex.ApplyOnce' for physical_address and capab…

00812d8

…ilities

Add info about 'capabilitiesManagedBySatellite'

25ee3a3

Add info about 'capabilitiesManagedBySatellite'

4f940a4

Add RequiredWith(physical_address) for capabilities

6380280

FishAndFrog commented Jul 30, 2024

View reviewed changes

hkantare approved these changes Aug 2, 2024

View reviewed changes

hkantare merged commit 52b77de into IBM-Cloud:master Aug 2, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add PhysicalAddress and CapabilitiesManagedBySatellite to Terraform SatelliteLocation for both resource and data-source #5530

Add PhysicalAddress and CapabilitiesManagedBySatellite to Terraform SatelliteLocation for both resource and data-source #5530

FishAndFrog commented Jul 25, 2024 •

edited

Loading

FishAndFrog commented Jul 25, 2024

hkantare Jul 30, 2024

FishAndFrog Jul 30, 2024

hkantare Jul 30, 2024

FishAndFrog Jul 30, 2024

TwoDCube Jul 30, 2024

FishAndFrog Jul 30, 2024

TwoDCube Jul 30, 2024

TwoDCube Jul 30, 2024

FishAndFrog Jul 30, 2024

FishAndFrog left a comment •

edited

Loading

Add PhysicalAddress and CapabilitiesManagedBySatellite to Terraform SatelliteLocation for both resource and data-source #5530

Add PhysicalAddress and CapabilitiesManagedBySatellite to Terraform SatelliteLocation for both resource and data-source #5530

Conversation

FishAndFrog commented Jul 25, 2024 • edited Loading

Community Note

FishAndFrog commented Jul 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FishAndFrog left a comment • edited Loading

Choose a reason for hiding this comment

FishAndFrog commented Jul 25, 2024 •

edited

Loading

FishAndFrog left a comment •

edited

Loading