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.

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

[AVM Module Issue]: Set-AVMModule updates README.md with wrong sort order #1930

Closed
1 task done
Agazoth opened this issue May 14, 2024 · 6 comments · Fixed by #1976
Closed
1 task done

[AVM Module Issue]: Set-AVMModule updates README.md with wrong sort order #1930

Agazoth opened this issue May 14, 2024 · 6 comments · Fixed by #1976
Assignees
Labels
Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue Type: Bug 🐛 Something isn't working

Comments

@Agazoth
Copy link
Contributor

Agazoth commented May 14, 2024

Check for previous/existing GitHub issues

  • I have checked for previous/existing GitHub issues

Issue Type?

Bug

Module Name

avm/res/storage/storage-account

(Optional) Module Version

No response

Description

When running Set-AVMModule -ModuleFolderPath .\avm\res\storage\storage-account\ -Recurse locally, the README.md file gets updated with a wrong sort order on the allowedCopyScope parameter.
image

(Optional) Correlation Id

No response

@Agazoth Agazoth added Needs: Triage 🔍 Maintainers need to triage still Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue labels May 14, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Type: Bug 🐛 Something isn't working label May 14, 2024
Copy link

@Agazoth, thanks for submitting this issue for the avm/res/storage/storage-account module!

Important

A member of the @Azure/avm-res-storage-storageaccount-module-owners-bicep or @Azure/avm-res-storage-storageaccount-module-contributors-bicep team will review it soon!

@avm-team-linter avm-team-linter bot added the Class: Resource Module 📦 This is a resource module label May 14, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Status: Response Overdue 🚩 When an issue/PR has not been responded to for X amount of days label May 17, 2024
@AlexanderSehr AlexanderSehr assigned AlexanderSehr and unassigned fblix May 19, 2024
@AlexanderSehr
Copy link
Contributor

Hey @Agazoth,
thanks for raising this. I'll look into this issue.
Before I do, one question though - The order of what is wrong? 😄 Is it about the 'AllowedValues' in your screenshot? Just want to make sure I look at the correct spot 😉

@AlexanderSehr AlexanderSehr removed Needs: Triage 🔍 Maintainers need to triage still Status: Response Overdue 🚩 When an issue/PR has not been responded to for X amount of days Class: Resource Module 📦 This is a resource module labels May 19, 2024
@Agazoth
Copy link
Contributor Author

Agazoth commented May 19, 2024

Hey @Agazoth, thanks for raising this. I'll look into this issue. Before I do, one question though - The order of what is wrong? 😄 Is it about the 'AllowedValues' in your screenshot? Just want to make sure I look at the correct spot 😉

Thanks, Alexander.

Before running the Set-AVMModule command, 'AAD' was on line 2280 and 'PrivateLink' on line 2381. The screenshot is of those two lines after running the command.

There is a rule for keeping parameters in correct sort order. If I push the wrong sort order, the pipeline fails. If I switch the two parameters after running the Set-AVMModule command and push the code, the pipeline does not complain.

@AlexanderSehr
Copy link
Contributor

Gotcha, thank you @Agazoth - I'll work on this as soon as I find a moment. Non-deterministic results can be a pretty big issue - hence the sorting.

@Agazoth
Copy link
Contributor Author

Agazoth commented May 19, 2024

I just realized, that this might be caused by my Danish regional settings. In Danish AA is equivalent to Å, which is the last letter in our alphabet.

If the sort order is fixed to en-us, that ought to solve the issue.

@AlexanderSehr
Copy link
Contributor

AlexanderSehr commented May 19, 2024

Hey @Agazoth, coincidentally, I just wanted to link this to an issue from the original CARML CI: Azure/ResourceModules#3722

Given that you essentially just confirmed the same, this will make solving the issue a lot faster 😄 Thanks for the update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue Type: Bug 🐛 Something isn't working
Projects
3 participants