Support Azure AD auth for Cosmos DB #1104
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
22 tasks
|
@ItalyPaleAle happy to see it, please fix CI. |
|
@daixiang0 the failed CI is not due to this PR, as I did not touch MQTT. Not sure if it's a previous commit that caused the error or just some flakiness in the test. |
|
For this case, you can re-commit to trigger CI re-run :) |
|
@CodeMonkeyLeet I was thinking this could be tracked as part of the umbrella issue #1103. Do you want me to create a separate issue for each one? |
#1103 tracks the general issue to be fixed in dapr/components-contrib, I'm asking for an issue in dapr/docs (or better yet, the documentation PR in dapr/docs) per:
The PR template includes this item because Dapr would like the documentation to be up-to-date with the new features included in each release (in this case, the ability to use Azure AD auth with the CosmosDB) and that's at the granularity of what has been merged into the release. |
|
Sorry I missed the part where you were referring to docs. I will open an issue. |
This was referenced Aug 31, 2021
CodeMonkeyLeet
approved these changes
Sep 1, 2021
artursouza
approved these changes
Sep 1, 2021
Codecov Report
@@ Coverage Diff @@
## master #1104 +/- ##
==========================================
- Coverage 34.53% 33.78% -0.75%
==========================================
Files 132 136 +4
Lines 10870 11400 +530
==========================================
+ Hits 3754 3852 +98
- Misses 6736 7155 +419
- Partials 380 393 +13
Continue to review full report at Codecov.
|
CodeMonkeyLeet
pushed a commit
to CodeMonkeyLeet/components-contrib
that referenced
this pull request
Sep 2, 2021
PR dapr#1104 introduced a regression in cosmosdb state component `Init()` by creating a `NewConfig()` which sets a new DefaultIdentificationHydrator, which causes a panic later when invoked. Patch this by restoring the configuration setting to use a struct with `nil` `IdentificationHydrator`.
3 tasks
artursouza
pushed a commit
that referenced
this pull request
Sep 2, 2021
PR #1104 introduced a regression in cosmosdb state component `Init()` by creating a `NewConfig()` which sets a new DefaultIdentificationHydrator, which causes a panic later when invoked. Patch this by restoring the configuration setting to use a struct with `nil` `IdentificationHydrator`.
CodeMonkeyLeet
pushed a commit
to CodeMonkeyLeet/components-contrib
that referenced
this pull request
Sep 3, 2021
Follow-up to dapr#1104 to support AD Auth codepath as well. `NewConfig()` and `NewConfigWithServicePrincipal()` both set the `Config.IdentificationHydrator` to the `DefaultIdentificationHydrator` in the underlying a8m/documentdb library. That implementation ends up calling `reflect.Value.Elem.FieldByName()`, which requires that the value passed to it is an interface or pointer, otherwise the Elem() call fails to dereference which causes a panic. cosmosdb.go passes the input to `UpsertDocument` by value today, which is eventually passed to the `DefaultIdentificationHydrator`, so changing the value passed to a pointer resolves the failure.
3 tasks
dapr-bot
added a commit
that referenced
this pull request
Sep 3, 2021
) Follow-up to #1104 to support AD Auth codepath as well. `NewConfig()` and `NewConfigWithServicePrincipal()` both set the `Config.IdentificationHydrator` to the `DefaultIdentificationHydrator` in the underlying a8m/documentdb library. That implementation ends up calling `reflect.Value.Elem.FieldByName()`, which requires that the value passed to it is an interface or pointer, otherwise the Elem() call fails to dereference which causes a panic. cosmosdb.go passes the input to `UpsertDocument` by value today, which is eventually passed to the `DefaultIdentificationHydrator`, so changing the value passed to a pointer resolves the failure. Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>
amimimor
pushed a commit
to amimimor/components-contrib
that referenced
this pull request
Dec 9, 2021
* Support Azure AD auth for Cosmos DB * Fixed linting errors * Tidying go.sum * Removed the need for nolint:shadow Co-authored-by: Simon Leet <31784195+CodeMonkeyLeet@users.noreply.github.com> Co-authored-by: Artur Souza <artursouza.ms@outlook.com> Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>
amimimor
pushed a commit
to amimimor/components-contrib
that referenced
this pull request
Dec 9, 2021
PR dapr#1104 introduced a regression in cosmosdb state component `Init()` by creating a `NewConfig()` which sets a new DefaultIdentificationHydrator, which causes a panic later when invoked. Patch this by restoring the configuration setting to use a struct with `nil` `IdentificationHydrator`.
amimimor
pushed a commit
to amimimor/components-contrib
that referenced
this pull request
Dec 9, 2021
…pr#1121) Follow-up to dapr#1104 to support AD Auth codepath as well. `NewConfig()` and `NewConfigWithServicePrincipal()` both set the `Config.IdentificationHydrator` to the `DefaultIdentificationHydrator` in the underlying a8m/documentdb library. That implementation ends up calling `reflect.Value.Elem.FieldByName()`, which requires that the value passed to it is an interface or pointer, otherwise the Elem() call fails to dereference which causes a panic. cosmosdb.go passes the input to `UpsertDocument` by value today, which is eventually passed to the `DefaultIdentificationHydrator`, so changing the value passed to a pointer resolves the failure. Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Updated the
bindings/azure/cosmosdbandstate/azure/cosmosdbcomponents to support auth via Azure AD.If
masterKeyis present in the metadata, that's used first. Otherwise, it tries authenticating using Azure AD, including using a service principal (azureClientIdandazureClientSecret), a certificate, or MSI.Issue reference
See #1103
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: