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

Add Lint ARM Templates step #2091

Merged
merged 33 commits into from
Apr 24, 2024
Merged

Add Lint ARM Templates step #2091

merged 33 commits into from
Apr 24, 2024

Conversation

romulets
Copy link
Member

@romulets romulets commented Apr 4, 2024

Summary of your changes

Add ARM TTK Linter to the github workflow and fix linter errors

Test results available on #1663 (comment)

Related Issues

Copy link

mergify bot commented Apr 4, 2024

This pull request does not have a backport label. Could you fix it @romulets? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit
    NOTE: backport-skip has been added to this pull request.

@mergify mergify bot added the backport-skip label Apr 4, 2024
Copy link

github-actions bot commented Apr 4, 2024

📊 Allure Report - 💚 No failures were reported.

Result Count
🟥 Failed 0
🟩 Passed 359
⬜ Skipped 33

Copy link
Member

@kubasobon kubasobon left a comment

Choose a reason for hiding this comment

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

LGTM when the templates are tested

.github/workflows/arm-template-lint.yml Outdated Show resolved Hide resolved
@romulets romulets force-pushed the arm-ttk branch 3 times, most recently from afaf7b7 to 820b8e1 Compare April 22, 2024 11:41
Comment on lines -220 to -237
{
"type": "Microsoft.Compute/virtualMachines/extensions",
"apiVersion": "2021-04-01",
"name": "cloudbeatVM/EnableMSIExtension",
"location": "[resourceGroup().location]",
"dependsOn": [
"cloudbeatVM"
],
"properties": {
"publisher": "Microsoft.ManagedIdentity",
"type": "ManagedIdentityExtensionForLinux",
"typeHandlerVersion": "1.0",
"autoUpgradeMinorVersion": true,
"settings": {
"port": 50342
}
}
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean this is was not necessary before?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was. I discussed with @jeniawhite and it was there to grant access to the graph API.

The Microsoft.Compute/virtualMachines/extensions Microsoft.ManagedIdentity has been deprecated and should not be used. The only place I found a deprecation was on azure cli release notes. I don't know the reason nor could find a documentation for a replacement. But still, that is a rule in the linter.

Through reading around (and some assumptions), it seems that the

"identity": {
  "type": "SystemAssigned"
}

Should suffice nowadays giving a managed identity to the VM.

I tested and verified everything works as expected (tests)

@romulets romulets requested a review from orestisfl April 24, 2024 08:25
@romulets romulets merged commit 7441d35 into elastic:main Apr 24, 2024
26 checks passed
Copy link
Contributor

@orestisfl orestisfl left a comment

Choose a reason for hiding this comment

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

(minor nitpick)

"type": "string",
"defaultValue": "cloudbeat",
"metadata": {
"description": "Admin username for the OS profile (Don't change)"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is in a nested deployment so I am not sure how users could potentially change that other than by editing the template.

@romulets romulets deleted the arm-ttk branch August 14, 2024 09:36
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.

[CIS Azure] Investigate using Azure Resource Manager Template Toolkit (arm-ttk)
3 participants