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

azure - custodian and c7n-org azure multi cloud bug fixes #6614

Merged
merged 15 commits into from
Apr 26, 2021

Conversation

d-van
Copy link
Contributor

@d-van d-van commented Apr 7, 2021

This will resolve issues with using multi-cloud with azure for both custodian and c7n-org.

custodian will now pass in the correct credential scopes when specifying a different cloud.

c7n-org will now initialize the azure session properly and set the cloud endpoints.

For c7n-org requires users to add region: ${AzureCloudName} to the account yml file

ex:

subscriptions:
- name: subscription_name_here
  subscription_id: subscription_here
  region: AzureUSGovernment

Issue Link: #6613

d-van added 2 commits April 7, 2021 12:54
Adding credential scopes to the client args to support multi-cloud for azure
Allow azure gov to initialize a session and set the cloud endpoints based on region set on the subscription
@d-van d-van requested a review from kapilt as a code owner April 7, 2021 18:48
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Apr 7, 2021

CLA Signed

The committers are authorized under a signed CLA.

@d-van d-van changed the title custodian and c7n-org azure multi cloud bug fixes auzre - custodian and c7n-org azure multi cloud bug fixes Apr 7, 2021
@d-van d-van changed the title auzre - custodian and c7n-org azure multi cloud bug fixes azure - custodian and c7n-org azure multi cloud bug fixes Apr 7, 2021
…6613)

Legacy credentials already does the credential scope within the function.
@d-van
Copy link
Contributor Author

d-van commented Apr 7, 2021

I originally added credential_scopes to the klass client in the legacy session client code, but it doesn't need it as legacy_credentials does it already.

Not sure how to test out the legacy cred workflow, so any insight on this would be greatly appreciated.

@logachev
Copy link
Collaborator

logachev commented Apr 8, 2021

@d-van I believe we have very small amount of resources that use legacy, role-assignment should be the one iirc.. So if you can test some policy that leverages this resources, should be fine..

Also, can you please add some test for your change? Like initialize Azure with USGov region, create a client and ensure it uses proper creds?

tools/c7n_azure/c7n_azure/provider.py Outdated Show resolved Hide resolved
tools/c7n_org/c7n_org/cli.py Outdated Show resolved Hide resolved
d-van added 2 commits April 9, 2021 13:17
Move session initialization in c7n-org. Created a couple of tests for azure us gov, but skip during live as they won't work unless you have access to azure gov
@d-van d-van requested a review from logachev April 14, 2021 20:55
@logachev
Copy link
Collaborator

Sorry for the delay on my side.

I think lack of provider initialization is a separate issue that we should address for all clouds.
In the meantime, I think Azure has an issue here: https://github.com/cloud-custodian/cloud-custodian/blob/master/tools/c7n_azure/c7n_azure/provider.py#L53

If options are provided, it should be using whatever is in the options rather than anything else.
So my guess is if you remove explicit initialization from c7n-org and update get_session_factory to use options.region if options.region else AZURE_PUBLIC_CLOUD - it should work now. Do you mind trying this?

@d-van
Copy link
Contributor Author

d-van commented Apr 23, 2021

@logachev that was it. I made the updates and it works for all cases (c7n commercial + gov and c7n-org commercial + gov)

Let me know if there is anything else I need to do! Thanks!

@logachev logachev merged commit b4881c5 into cloud-custodian:master Apr 26, 2021
@d-van
Copy link
Contributor Author

d-van commented Apr 26, 2021

@logachev let us know if we can help out develop and test out anything relating to azure gov. When azure secret comes out, I think we will have resources to test the capabilities out.

@logachev
Copy link
Collaborator

@d-van Thanks! Do you mean Azure KeyVault Secret?

@kapilt
Copy link
Collaborator

kapilt commented Apr 26, 2021

i think that was in reference to another sovereign cloud endpoint

@d-van
Copy link
Contributor Author

d-van commented Apr 26, 2021

@kapilt @logachev yep! Whenever Azure Secret Region gets released we'll have to add those cloud endpoints to ensure users can use cloud custodian in the secret/top secret regions.

@d-van
Copy link
Contributor Author

d-van commented Apr 26, 2021

If I recall correctly, some/most of the endpoints may be classified so we may need to allow custom endpoint configurations. I guess we'll pick this back up whenever we have to cross the azure secret region bridge.

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.

3 participants