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

fix: Update parameters to implement non-AAD integrated clusters in Kubernetes #3828

Merged
merged 20 commits into from
Dec 11, 2024

Conversation

Menghua1
Copy link
Member

@Menghua1 Menghua1 commented Nov 22, 2024

Description

This PR introduces a conditional check for aadProfile configuration in Kubernetes cluster settings. Adds a user-defined type for the aadProfile parameter, and when the aadProfile parameter is empty, it disables AAD (Azure Active Directory). Ensures that AAD integration is completely skipped when not needed, optimizing resource usage and configuration complexity.

Requested by the AZD team: Azure/Azure-Verified-Modules#261, to ensure consistency with the functionality implemented in the aks-managed-cluster.bicep file located in infra/core.

Pipeline Reference

Pipeline
avm.res.container-service.managed-cluster

Type of Change

  • Update to CI Environment or utilities (Non-module affecting changes)
  • Azure Verified Module updates:
    • Bugfix containing backwards-compatible bug fixes, and I have NOT bumped the MAJOR or MINOR version in version.json:
      • Someone has opened a bug report issue, and I have included "Closes #{bug_report_issue_number}" in the PR description.
      • The bug was found by the module author, and no one has opened an issue to report it yet.
    • Feature update backwards compatible feature updates, and I have bumped the MINOR version in version.json.
    • Breaking changes and I have bumped the MAJOR version in version.json.
    • Update to documentation

Checklist

  • I'm sure there are no other open Pull Requests for the same update/change
  • I have run Set-AVMModule locally to generate the supporting module files.
  • My corresponding pipelines / checks run clean and green without any errors or warnings

@rajeshkamal5050 for notification.

@Menghua1 Menghua1 requested review from a team as code owners November 22, 2024 04:32
@avm-team-linter avm-team-linter bot added the Needs: Module Owner 📣 This module needs an owner to develop or maintain it label Nov 22, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added Needs: Triage 🔍 Maintainers need to triage still Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue labels Nov 22, 2024
update main.json

update main.json

update main.json

update main.json

update main.json
@Menghua1 Menghua1 force-pushed the fix/add-aad-profile-conditional branch from 113393b to 71933fd Compare November 22, 2024 04:34
@Menghua1
Copy link
Member Author

@AlexanderSehr Could you please review this PR?

@AlexanderSehr
Copy link
Contributor

@AlexanderSehr Could you please review this PR?

Hey @Menghua1,
I can - but it's the module owners @ilhaan & @JPEasier that would be the ones that need to approve the PR.

Also, there is a very blocking PR here. Without that being resolved, we cannot move forward as this PR. If merged regardless, it will fail the deployment tests in AVM and hence never make it to the publish stage.

@Menghua1 Menghua1 force-pushed the fix/add-aad-profile-conditional branch from 4adfaac to 063c8c5 Compare November 27, 2024 09:54
@Menghua1 Menghua1 changed the title fix: Add enableAad parameter to conditionally configure AAD profile in Kubernetes fix: Update parameters to implement non-AAD integrated clusters in Kubernetes Nov 28, 2024
@JPEasier JPEasier added the Needs: Author Feedback 👂 Awaiting feedback from the issue/PR author label Nov 29, 2024
@Menghua1 Menghua1 force-pushed the fix/add-aad-profile-conditional branch from 198cca6 to 4253e3e Compare November 29, 2024 09:06
@AlexanderSehr AlexanderSehr marked this pull request as draft November 29, 2024 11:09
@AlexanderSehr AlexanderSehr marked this pull request as ready for review November 29, 2024 11:09
@v-xuto
Copy link
Member

v-xuto commented Dec 2, 2024

@AlexanderSehr If there are no issues with this PR review, please help merge this PR. Thanks a lot.

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs: Attention 👋 Reply has been added to issue, maintainer to review and removed Needs: Author Feedback 👂 Awaiting feedback from the issue/PR author labels Dec 2, 2024
@AlexanderSehr
Copy link
Contributor

@AlexanderSehr If there are no issues with this PR review, please help merge this PR. Thanks a lot.

Not from my end, but I'll leave the final review to the module's owner @JPEasier :)

@ilhaan
Copy link
Member

ilhaan commented Dec 4, 2024

@AlexanderSehr Is #3646 no longer blocking PR?

@v-xuto
Copy link
Member

v-xuto commented Dec 5, 2024

@JPEasier Could you please review this PR? If there is no problem, please merge them. Thank a lot.

@AlexanderSehr
Copy link
Contributor

@AlexanderSehr Is #3646 no longer blocking PR?

true, true. Thanks for pointing this out @ilhaan

@rajeshkamal5050
Copy link

rajeshkamal5050 commented Dec 11, 2024

@JPEasier can you help with merging this PR? cc @matebarabas @jongio

@ilhaan ilhaan merged commit 75bf761 into Azure:main Dec 11, 2024
16 checks passed
AlexanderSehr pushed a commit that referenced this pull request Jan 7, 2025
…n Kubernetes (#3928)

## Description
Based on PR #3828
update to `avm/res/container-service/managed-cluster`, add `aadProfile`
parameter to control AAD integration.
<!--
>Thank you for your contribution !
> Please include a summary of the change and which issue is fixed.
> Please also include the context.
> List any dependencies that are required for this change.

Fixes #123
Fixes #456
Closes #123
Closes #456
-->

## Pipeline Reference

<!-- Insert your Pipeline Status Badge below -->

| Pipeline |
| -------- |
|
[![avm.ptn.azd.aks](https://github.com/Menghua1/bicep-registry-modules/actions/workflows/avm.ptn.azd.aks.yml/badge.svg)](https://github.com/Menghua1/bicep-registry-modules/actions/workflows/avm.ptn.azd.aks.yml)
|

## Type of Change

<!-- Use the checkboxes [x] on the options that are relevant. -->

- [ ] Update to CI Environment or utilities (Non-module affecting
changes)
- [x] Azure Verified Module updates:
- [ ] Bugfix containing backwards-compatible bug fixes, and I have NOT
bumped the MAJOR or MINOR version in `version.json`:
- [ ] Someone has opened a bug report issue, and I have included "Closes
#{bug_report_issue_number}" in the PR description.
- [ ] The bug was found by the module author, and no one has opened an
issue to report it yet.
- [ ] Feature update backwards compatible feature updates, and I have
bumped the MINOR version in `version.json`.
- [ ] Breaking changes and I have bumped the MAJOR version in
`version.json`.
  - [ ] Update to documentation

## Checklist

- [x] I'm sure there are no other open Pull Requests for the same
update/change
- [x] I have run `Set-AVMModule` locally to generate the supporting
module files.
- [x] My corresponding pipelines / checks run clean and green without
any errors or warnings

<!-- Please keep up to date with the contribution guide at
https://aka.ms/avm/contribute/bicep -->

@rajeshkamal5050 for notification.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Attention 👋 Reply has been added to issue, maintainer to review Needs: Module Owner 📣 This module needs an owner to develop or maintain it Needs: Triage 🔍 Maintainers need to triage still Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants