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

Consistency issue between pulumi-azuread & pulumi-azure? #38

Open
juchom opened this issue Oct 30, 2019 · 6 comments
Open

Consistency issue between pulumi-azuread & pulumi-azure? #38

juchom opened this issue Oct 30, 2019 · 6 comments
Labels
kind/enhancement Improvements or new features

Comments

@juchom
Copy link

juchom commented Oct 30, 2019

I'm currently working with both packages in my solution and according to the pulumi documentation the configuration for each package use the following variables:

pulumi-azure

  • azure:clientId or ARM_CLIENT_ID
  • azure:subscriptionId or ARM_SUBSCRIPTION_ID
  • azure:tenantId or ARM_TENANT_ID
  • azure:clientSecret or ARM_CLIENT_SECRET

pulumi-azuread

  • azuread:clientId or ARM_CLIENT_ID
  • azuread:subscriptionId or ARM_SUBSCRIPTION_ID
  • azuread:tenantId or ARM_TENANT_ID
  • azuread:clientSecret or ARM_CLIENT_SECRET
  • azuread:certificatePassword or ARM_CLIENT_CERTIFICATE_PASSWORD
  • azuread:clientCertificatePath or ARM_CLIENT_CERTIFICATE_PATH
  • azuread:environment or ARM_ENVIRONMENT

Would it make sense to rename azuread:* to azure:* because they use the same environment variables?

@stack72
Copy link
Contributor

stack72 commented Nov 17, 2019

Hi @juchom

you are correct - each of the packages don't reflect the same configuration variables and they both should. I have opened a PR in our tfgen generation repo that creates the configuration to ensure we pick this validation up in the future

With regards to using the same azure:*, technically these are different providers therefore they do need to be configured differently. When the PR (linked) is merged, at least the providers will use the same consistent environment variables

Thoughts?

Paul

@stack72
Copy link
Contributor

stack72 commented Nov 17, 2019

@stack72 stack72 added the bug label Nov 17, 2019
@stack72 stack72 added this to the 0.29 milestone Nov 17, 2019
@juchom
Copy link
Author

juchom commented Nov 18, 2019

Hi @stack72 ,

I'm not sure to understand how things will change after your PR?

Could you please give an example of the config before -> after.

Julien

@stack72
Copy link
Contributor

stack72 commented Nov 18, 2019

Hi @juchom

The issue is, we are missing some fields that are not mapped in the azure provider. Therefore when this PR gets merged we will be alerted to those and then we can roll those changes out

This PR helps us understand the underlying schema better so we are fully covered

Paul

@juchom
Copy link
Author

juchom commented Nov 18, 2019

Ok, when I opened this issue, I had this in mind:

Imagine you use the azure & azuread provider in your stack, we can do that with config variables:

  • azure:clientId 1234
  • azuread:clientId 5678

But if we use the environment variable we only have ARM_CLIENT_ID for both.

I see your point to keep azure:* and azuread:*, but in that case should we have ARM_CLIENT_ID and ARM_AD_CLIENT_ID for azuread provider?

@stack72 stack72 modified the milestones: 0.29, 0.30 Nov 19, 2019
@stack72 stack72 modified the milestones: 0.30, 0.31 Dec 18, 2019
@stack72 stack72 modified the milestones: 0.31, 0.32 Jan 13, 2020
@pgavlin pgavlin modified the milestones: 0.32, 0.33 Feb 12, 2020
@leezen leezen removed this from the 0.33 milestone Mar 9, 2020
@infin8x infin8x added kind/bug Some behavior is incorrect or out of spec and removed bug labels Jul 10, 2021
@nimro
Copy link

nimro commented Sep 16, 2024

With regards to using the same azure:*, technically these are different providers therefore they do need to be configured differently.

I see your point to keep azure:* and azuread:*, but in that case should we have ARM_CLIENT_ID and ARM_AD_CLIENT_ID for azuread provider?

I'd love this. Having them share the same env vars is a massive pain when trying to configure them to access different tenants. Instead of CI simply setting appropriate env vars for both, one has to jump through hoops.

Even an optional dedicated set of AD env vars that are checked before the default "ARM" ones would be fantastic, and still provide backwards compatibility.

@thomas11 thomas11 added kind/enhancement Improvements or new features and removed kind/bug Some behavior is incorrect or out of spec labels Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements or new features
Projects
None yet
Development

No branches or pull requests

7 participants