Skip to content

Workload Identity support#266

Merged
lonegunmanb merged 3 commits intoAzure:masterfrom
nlamirault:feat/workload-identity
Oct 18, 2022
Merged

Workload Identity support#266
lonegunmanb merged 3 commits intoAzure:masterfrom
nlamirault:feat/workload-identity

Conversation

@nlamirault
Copy link
Copy Markdown
Contributor

@nlamirault nlamirault commented Oct 14, 2022

Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
Copy link
Copy Markdown
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 @nlamirault for opening this pr. We're introduced a CI pipeline for this module and now we're enforcing some code style policies. You can check whether your pr meets the requirement by reading this document and run the make pr-check command. Thanks for your understanding.

Comment thread variables.tf Outdated
default = false
}

variable "workload_identity_enabled" {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we reorder variables in alphabet order please? Thanks.

Comment thread main.tf Outdated
local_account_disabled = var.local_account_disabled
node_resource_group = var.node_resource_group
oidc_issuer_enabled = var.oidc_issuer_enabled
workload_identity_enabled = var.workload_identity_enabled
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we reorder arguments in alphabet order please? Thanks.

Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
@nlamirault nlamirault temporarily deployed to acctests October 17, 2022 07:41 Inactive
Copy link
Copy Markdown
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 @nlamirault , LGTM.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants