Skip to content

list and identity implementation : azurerm_storage_sync_server_endpoint#31988

Open
aayushsingh2502 wants to merge 4 commits intomainfrom
strgsyncserver_identity_list
Open

list and identity implementation : azurerm_storage_sync_server_endpoint#31988
aayushsingh2502 wants to merge 4 commits intomainfrom
strgsyncserver_identity_list

Conversation

@aayushsingh2502
Copy link
Copy Markdown
Contributor

@aayushsingh2502 aayushsingh2502 commented Mar 18, 2026

Community Note

  • Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
  • Please do not leave comments along the lines of "+1", "me too" or "any updates", they generate extra noise for PR followers and do not help prioritize for review

Description

Implemented list and identity in azurerm_storage_sync_server_endpoint
Teamcity link: https://hashicorp.teamcity.com/buildConfiguration/TF_AzureRM_AZURERM_SERVICE_PUBLIC_STORAGE/631915?buildTab=overview

Manul run output:

aayushsingh@Aayushs-MacBook-Pro test % terraform query -var-file=terraform.tfvars -var 'storage_sync_group_id=/subscriptions/1a6092a6-137e-4025-9a7c-ef77f76f2c02/resourceGroups/acctestRG-StorageSync-306329/providers/Microsoft.StorageSync/storageSyncServices/acctest-StorageSync-306329/syncGroups/acctest-StorageSyncGroup-306329'
╷
│ Warning: Provider development overrides are in effect
│ 
│ The following provider development overrides are set in the CLI configuration:
│  - aayushsingh/learnings/ymusic in /Users/aayushsingh/Projects/learnings/provider/terraform-provider-ymusic
│  - hashicorp/azurerm in /Users/aayushsingh/terraform-provider-azurerm
│ 
│ The behavior may therefore not match any released version of the provider and applying changes may cause the
│ state to become incompatible with published releases.
╵
list.azurerm_storage_sync_server_endpoint.by_sync_group   name=acctestSE-306329,resource_group_name=acctestRG-StorageSync-306329,storage_sync_service_name=acctest-StorageSync-306329,subscription_id=1a6092a6-137e-4025-9a7c-ef77f76f2c02,sync_group_name=acctest-StorageSyncGroup-306329   acctestSE-306329

PR Checklist

  • I have followed the guidelines in our Contributing Documentation.
  • I have checked to ensure there aren't other open Pull Requests for the same update/change.
  • I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
  • I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source.
  • I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.
    For example: “resource_name_here - description of change e.g. adding property new_property_name_here

Changes to existing Resource / Data Source

  • I have added an explanation of what my changes do and why I'd like you to include them (This may be covered by linking to an issue above, but may benefit from additional explanation).
  • I have written new tests for my resource or datasource changes & updated any relevant documentation.
  • I have successfully run tests with my changes locally. If not, please provide details on testing challenges that prevented you running the tests.
  • (For changes that include a state migration only). I have manually tested the migration path between relevant versions of the provider.

Testing

  • My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

  • azurerm_resource - support for the thing1 property [GH-00000]

This is a (please select all that apply):

  • Bug Fix
  • New Feature (ie adding a service, resource, or data source)
  • Enhancement
  • Breaking Change

Related Issue(s)

Fixes #0000

AI Assistance Disclosure

  • AI Assisted - This contribution was made by, or with the assistance of, AI/LLMs

Rollback Plan

If a change needs to be reverted, we will publish an updated version of the provider.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

Note

If this PR changes meaningfully during the course of review please update the title and description as required.

)

type StorageSyncServerEndpointTestResource struct{}
type StorageSyncServerEndpointResource struct{}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why rename it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The rename was done as part of the refactor that introduced the sequential wrapper test and reused the helper across multiple test files. The file moved from standalone tests to grouped sequential tests, and the helper struct name was normalized from StorageSyncServerEndpointTestResource to StorageSyncServerEndpointResource.

@rigalGit
Copy link
Copy Markdown
Contributor

rigalGit commented Mar 18, 2026

Added some minor comments except testing one. Also documentation changes are not present, please add them also.

Copilot cli also caught a bug in list acceptance test query. Pasting co-pilot's code review comments also.

Main findings:

  • internal/services/storage/storage_sync_server_endpoint_resource_list_test.go:44 has a broken query config. basicListQuery() references azurerm_storage_sync_group.test.id, but that resource is not declared in
    that step’s config, so the query step will fail if unskipped.
  • Acceptance coverage is effectively disabled. Both TestAccStorageSyncServerEndpointSequential and TestAccStorageSyncServerEndpoint_list_basic call t.Skip(...), so the generated identity test never actually
    runs.
  • The list-resource docs appear missing. I couldn’t find website/docs/list-resources/storage_sync_server_endpoint.html.markdown, which the list guide calls for.

@aayushsingh2502
Copy link
Copy Markdown
Contributor Author

  • The list-resource docs appear missing. I couldn’t find website/docs/list-resources/storage_sync_server_endpoint.html.markdown, which the list guide calls for.

Fixed, thanks for flagging this.
storage_sync_server_endpoint_resource_list_test.go had an invalid query config reference ([azurerm_storage_sync_group.test.id in the query-only step. I updated the test to pass a concrete storage_sync_group_id string built from test data, so the query step no longer depends on undeclared resources and runs correctly when enabled.

Added.
Created the missing list-resource documentation at storage_sync_server_endpoint.html.markdown], including example usage and argument reference for storage_sync_group_id. I also ran docs validation and it passes.

@toddgiguere toddgiguere self-assigned this Apr 10, 2026
@toddgiguere
Copy link
Copy Markdown
Contributor

@rigalGit have all your previous issues been addressed? I've looked at this myself and I think it is good.

@aayushsingh2502 can you resolve the current conflict and run the tests manually with a registered server? The TC test link above wasn't really a test, all tests are ignored (skip), if there is a way to run the tests manually and post the summary of results here, that would be good.

@aayushsingh2502
Copy link
Copy Markdown
Contributor Author

@toddgiguere I have fixed the merge conflict.
I also have added the output of manual test in he description under Manul run output.
Please let me know if anything else is required

@toddgiguere
Copy link
Copy Markdown
Contributor

toddgiguere commented Apr 16, 2026

@aayushsingh2502 I wasn't referring to a manual run of the query, I meant a manual run of the actual unit tests, especially since you added identity and made changes to the existing endpoint_test.go.

If you look at your TeamCity link in the description, all of the tests that were run are just the following output:

--- SKIP: TestAccStorageSyncServerEndpoint_list_basic (0.00s)

That's because there is a manual step required to run the tests, so they are skipped.

Are you able to run the whole unit test set manually? If so that is what I meant.

@aayushsingh2502
Copy link
Copy Markdown
Contributor Author

@toddgiguere
I ran the test manually in 2 phases. Following is the detail for it:

Manual Test Summary: azurerm_storage_sync_server_endpoint

What was tested

  1. Phase 1 infra + registration flow (terraform apply -auto-approve).
    • Provisioned VM + Storage Sync prerequisites (sync service/group + cloud endpoint).
    • Used a single Windows CustomScriptExtension to run install + registration sequence.
    • Installed Azure File Sync agent MSI on the VM.
    • Installed/imported Az.Accounts and Az.StorageSync PowerShell modules.
    • Authenticated via user-assigned managed identity (Connect-AzAccount -Identity) and selected subscription context.
    • Performed server registration with Get-AzStorageSyncService + Register-AzStorageSyncServer (with retry handling).
    • Confirmed registration by checking registered_servers output and using that ID in phase 2.
  2. Phase 2 endpoint creation:
    • terraform apply -auto-approve -var='create_server_endpoint=true' -var='registered_server_id=.../registeredServers/ffa33815-e694-44ca-903b-d595e759fbbb'
  3. List query validation (terraform query -json ...).
  4. Identity/state validation (terraform output, terraform state show).

Key outcomes

  1. Phase 2 apply succeeded:
    • Apply complete! Resources: 1 added, 0 changed, 0 destroyed.
  2. Server endpoint created:
    • /subscriptions/1a6092a6-137e-4025-9a7c-ef77f76f2c02/resourceGroups/acctest-rg-afs-34045/providers/Microsoft.StorageSync/storageSyncServices/acctestsync34045/syncGroups/acctest-sync-group-34045/serverEndpoints/acctest-server-endpoint-34045
  3. Registered server used:
    • /subscriptions/1a6092a6-137e-4025-9a7c-ef77f76f2c02/resourceGroups/acctest-rg-afs-34045/providers/Microsoft.StorageSync/storageSyncServices/acctestsync34045/registeredServers/ffa33815-e694-44ca-903b-d595e759fbbb
  4. Query result confirmed list behavior:
    • list_complete.total = 1
    • identity fields returned: subscription_id=1a6092a6-137e-4025-9a7c-ef77f76f2c02, resource_group_name=acctest-rg-afs-34045, storage_sync_service_name=acctestsync34045, sync_group_name=acctest-sync-group-34045, name=acctest-server-endpoint-34045
  5. State includes expected endpoint fields:
    • name = acctest-server-endpoint-34045
    • registered_server_id = .../registeredServers/ffa33815-e694-44ca-903b-d595e759fbbb
    • server_local_path = C:\\SyncRoot

Conclusion

Manual validation passed. azurerm_storage_sync_server_endpoint creation, list query behavior, and identity surface all worked as expected in Azure.

Comment thread internal/services/storage/registration.go Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants