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 confidential computing in aks module #423

Merged
merged 2 commits into from
Aug 11, 2023

Conversation

jiaweitao001
Copy link
Collaborator

Describe your changes

Issue number

#421

Checklist before requesting a review

  • The pr title can be used to describe what this pr did in CHANGELOG.md file
  • I have executed pre-commit on my machine
  • I have passed pr-check on my machine

Thanks for your cooperation!

@jiaweitao001 jiaweitao001 temporarily deployed to acctests August 11, 2023 01:11 — with GitHub Actions Inactive
Copy link
Member

@lonegunmanb lonegunmanb left a comment

Choose a reason for hiding this comment

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

Thanks @jiaweitao001, one comment.

variables.tf Outdated
@@ -388,6 +388,14 @@ variable "cluster_name" {
description = "(Optional) The name for the AKS resources created in the specified Azure Resource Group. This variable overwrites the 'prefix' var (The 'prefix' var will still be applied to the dns_prefix if it is set)"
}

variable "confidential_computing" {
type = list(object({
Copy link
Member

Choose a reason for hiding this comment

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

According to the schema, could we set type to boolean for this variable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, we can simplify this type to a boolean, I was wondering there might be a possibility that more attributes will be added to this confidential_computing in the future, use a list would be more scalable?

Copy link
Member

Choose a reason for hiding this comment

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

Since the MaxItems of this list is 1 I think in that case, an object would be enough for us. WDYT?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Make sense. We can just use object instead of list.

@jiaweitao001 jiaweitao001 temporarily deployed to acctests August 11, 2023 05:51 — with GitHub Actions Inactive
Copy link
Member

@lonegunmanb lonegunmanb left a comment

Choose a reason for hiding this comment

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

Thanks @jiaweitao001, LGTM!

@lonegunmanb lonegunmanb merged commit 05e12ca into Azure:main Aug 11, 2023
@jiaweitao001 jiaweitao001 deleted the add_confidential_computing branch August 14, 2023 08:29
skolobov pushed a commit to skolobov/terraform-azurerm-aks that referenced this pull request Oct 29, 2023
* Add confidential computing in aks module
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants