[AVM Testing Workflows]: Custom Secrets in AVM E2E Pipeline - Azure Stack HCI Modules

[mbrat2005]

Check for previous/existing GitHub issues

• [X] I have checked for previous/existing GitHub issues

Description

In order to deploy an Azure Stack HCI cluster, an App Registration and client secret credential is currently required (until managed identity is supported). This is a blocker for the HCI cluster and related resource modules. We have looked into options to dynamically create this App Registration, but Deployment Scripts require excessive permissions for the GitHub action credential and Bicep MS Graph support doesn't allow creating secret credentials.

This has left passing an existing App Registration ID and secret through GitHub secrets to the E2E pipeline during execution. However, this doesn't appear possible today without modifying the template workflows.

This impacts these proposed modules: avm/res/azure-stack-hci/ cluster, virtual-hard-disk, virtual-machine-instance, network-interface. Others, which have yet to be proposed related to AKS and SDN on HCI, as well as pattern modules would also be blocked.

[mbilalamjad]

Tagging @Azure/avm-core-team-technical-bicep for attention

[AlexanderSehr]

This sounds familiar.

I replied to a similar question in another thread and at least on the Bicep side, by recommendation would be to implement a generic solution along the lines of

• Agree on a secret prefix like inject_
• Implement a logic in the pipeline that looks for secrets with that prefix and automatically passes them as parameters into the target deployment IF the used template supports that parameter (we can do this already using the AdditionalParameters parameter of both the Test-TemplateDeployment & New-TemplateDeployment scripts.
  • To make this a bit more pleasent to the eye, the parameter ideally does not have inject_ in the name, so we could implement something where a secret with the name inject_hciServicePrincipalSecret would expect a parameter hciServicePrincipalSecret (which should be declared with @secure()

This way, only users that have a secret configured will use it, and on the Upstream-side we just have to make sure that all secrets are configured in the environment before a PR expecting new secrets is merged. @mbrat2005, would that satisfy your requirements?

[mbrat2005]

Hi @AlexanderSehr, yes--that sounds like it would work for me!

[AlexanderSehr]

In that case @mbrat2005 & @mbilalamjad, I'd suggest to transfer this issue to the BRM repository and discuss the proposed solution with the technical maintainers to enable the feature's implementation :) Any objections?

[mbrat2005]

@matebarabas Thoughts on next steps here?

[matebarabas]

@AlexanderSehr, I transferred the issue to BRM as suggested. Can you please link any related, already existing issues to it? If I'm not mistaken this is already being discussed and the solution is being developed somewhere. Can you please confirm? Thanks!

CC: @mbrat2005

[AlexanderSehr]

Sure thing. This bad boy is already implemented as a PoC/MVP here and documented here. I think the overall flow won't change much before it's release, but the naming might (of secrets & the variable).

[AlexanderSehr]

Hey @mbrat2005, the custom-inputs/secrets feature was merged just recently via this PR #3015 and is documented here. If you agree, then I'd suggest to close the issue as completed :)

[mbrat2005]

Amazing, thanks for all the work @AlexanderSehr!

For the documentation piece, a quick section detailing the steps to be taken before attempting a PR to upstream main would be good--I'm assuming that we (module owners) need to provide you (maintainers) with the list of secrets to be added to the upstream key vault and any related configuration for those secret's target resources/identities?

[AlexanderSehr]

e documentation piece, a quick section detailing the steps to be taken before attempting a PR to upstream main would be good--I'm assuming that we (module owners) need to provide you (maintaine

That's correct @mbrat2005.

@jtracey93 actually just called this out today and we added a disclaimer on top of the docs. It's not perfect, but a start.

That being said, if there is any special configuration needed, I'd further suggest to add something to the test parameter's description, or add a comment in the test case that you could point in the PR to. This would help any potential contributor to set to module to replicate the setup too 💪

[mbrat2005]

@AlexanderSehr Giving this a test run today, it seems that the CI secrets solution may need something to work with PSRules, which expects parameters to have values or default values when it expands the bicep files.

Does that sound right to you? Do you think the param values need to be added to the module files before the PSRules testing runs? If so, I could see if I can get that working following the other CI variable code...

[AlexanderSehr]

Hey @mbrat2005, dang sorry, I thought I added that remark to the docs. Just opened this PR to add the missing piece. In short, you have to provide a default value (an empty string is enough) for the new parameter. This is because PSRule wants to expand the template and hence needs a value for every parameter. We intend to slightly alter the implementation in the future as there is an alternative way to do this (i.e., defining defaults in the PSRule configuration), that it's currently not working for us. We did however already create an issue for it and it will likely be fixed sooner than later. Until then we'll make due with the default value and then update the approach on behalf of the owners later :)

[mbrat2005]

Great, thanks! I'll try that

[mbrat2005]

Tested again and a default blank value workaround works.