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

[AVM CI Environment Issue]: Pester static tests for managedIdentitiesType are skipped #3895

Open
1 task done
peterbud opened this issue Dec 6, 2024 · 4 comments · May be fixed by #4112
Open
1 task done

[AVM CI Environment Issue]: Pester static tests for managedIdentitiesType are skipped #3895

peterbud opened this issue Dec 6, 2024 · 4 comments · May be fixed by #4112
Assignees
@AlexanderSehr
Labels
Needs: Core Team 🧞 This item needs the AVM Core Team to review it Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue Type: Bug 🐛 Something isn't working Type: CI 🚀 This issue is related to the AVM CI

Comments

@peterbud
Copy link
Contributor

peterbud commented Dec 6, 2024

Check for previous/existing GitHub issues

  • I have checked for previous/existing GitHub issues

Issue Type?

Bug

Description

I have noticed that Pester static tests for managedIdentitiesType are skipped in the CI runs, as I believe the type has been recently renamed to managedIdentitiesAllType.

Example from a recent CI run:
Module tests / Template tests / Parameters / Schema-based User-defined-types tests / [sql/server] If a UDT definition [managedIdentitiesType] exists and supports system-assigned-identities, the template should have an output for its principal ID. |

Test is skipped, because because the module template has no [managedIdentitiesType] UDT definition or does not support system-assigned-identities.
@peterbud peterbud added Needs: Core Team 🧞 This item needs the AVM Core Team to review it Needs: Triage 🔍 Maintainers need to triage still Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue Type: CI 🚀 This issue is related to the AVM CI labels Dec 6, 2024
@github-project-automation github-project-automation bot moved this to Needs: Triage in AVM - Issue Triage Dec 6, 2024
@microsoft-github-policy-service Microsoft GitHub Policy Service
Copy link

Important

The "Needs: Triage 🔍" label must be removed once the triage process is complete!

Tip

For additional guidance on how to triage this issue/PR, see the BRM Issue Triage documentation.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Type: Bug 🐛 Something isn't working label Dec 6, 2024
@microsoft-github-policy-service Microsoft GitHub Policy Service
Copy link

Warning

Tagging the AVM Core Team (@Azure/avm-core-team-technical-bicep) due to a module owner or contributor having not responded to this issue within 3 business days. The AVM Core Team will attempt to contact the module owners/contributors directly.

Tip

  • To prevent further actions to take effect, the "Status: Response Overdue 🚩" label must be removed, once this issue has been responded to.
  • To avoid this rule being (re)triggered, the ""Needs: Triage 🔍" label must be removed as part of the triage process (when the issue is first responded to)!

@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 Dec 12, 2024
@AlexanderSehr
Copy link
Contributor

AlexanderSehr commented Dec 14, 2024 •
edited
Loading

Hey @peterbud,
Very observant 💪
With the introduction of the avm-common-types we had to refactor the current tests to allow for references instead of local implementations and this may have fallen through the cracks.
That being said, there are plans to introduce a new set of tests for these types (for one, regarding the implementation + their version) but we should fix this regardless.

That being said, regarding the specific test in question it should be noted that it should only fire if the resource supports system assigned identities. We hence need to find a better/new way to detect this case.

cc: @eriqua (if you want to add anything 🙂)

Will remove the triage label. And we can work on this once a PR moving the utilities folder was merged (to avoid conflicts)

@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 labels Dec 14, 2024
@AlexanderSehr AlexanderSehr self-assigned this Dec 14, 2024
@AlexanderSehr AlexanderSehr linked a pull request Dec 30, 2024 that will close this issue
fix: Fixed test for System-Assigned Identity output #4112
Draft
8 tasks
@AlexanderSehr
Copy link
Contributor

AlexanderSehr commented Dec 30, 2024 •
edited
Loading

@peterbud & @eriqua, I just linked a PR that not only fixes the issue, but also validates the output more thoroughly & updates all current modules to that spec to not fail the new static tests (in total, 45 modules).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AlexanderSehr AlexanderSehr

Labels
Needs: Core Team 🧞 This item needs the AVM Core Team to review it Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue Type: Bug 🐛 Something isn't working Type: CI 🚀 This issue is related to the AVM CI
Projects
AVM - Issue Triage
Status: Needs: Triage
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: Fixed test for System-Assigned Identity output AlexanderSehr/bicep-registry-modules
2 participants
@AlexanderSehr @peterbud