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.

Sign up for GitHub

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

Jump to bottom

Cosmos Key Credential #4885

Merged
merged 4 commits into from
Aug 8, 2019
Merged

Cosmos Key Credential #4885

merged 4 commits into from
Aug 8, 2019

Conversation

kushagraThapar
Copy link
Member

  • New class CosmosKeyCredential.
  • CosmosClientBuilder takes CosmosKeyCredential.
  • Updating the key in the same instance of CosmosKeyCredential will reflect the update throughout the BaseAuthorizationTokenProvider.
  • RxDocumentClientImpl will wrap the master key in CosmosKeyCredential, if CosmosKeyCredential is not provided to support backward compatibility.

@kushagraThapar kushagraThapar self-assigned this Aug 7, 2019
@christopheranderson christopheranderson mentioned this pull request Aug 7, 2019
Open
@kushagraThapar
Copy link
Member Author

/azp run java-cosmos-tests

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@kushagraThapar
Copy link
Member Author

/azp run java - cosmos - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

christopheranderson
christopheranderson suggested changes Aug 7, 2019
View reviewed changes
Copy link
Contributor

@christopheranderson christopheranderson left a comment

Choose a reason for hiding this comment

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

I have some requested changes to public surface area, but I think implementation/test coverage looks good.

sdk/cosmos/microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/CosmosClient.java Outdated Show resolved Hide resolved
sdk/cosmos/microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/CosmosClientBuilder.java Show resolved Hide resolved
sdk/cosmos/microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/CosmosClientBuilder.java Show resolved Hide resolved
sdk/cosmos/microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/CosmosKeyCredential.java Outdated Show resolved Hide resolved
sdk/cosmos/microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/CosmosKeyCredential.java Outdated Show resolved Hide resolved
sdk/cosmos/microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/CosmosKeyCredential.java Show resolved Hide resolved
sdk/cosmos/microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/CosmosKeyCredential.java Outdated Show resolved Hide resolved
sdk/cosmos/microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/CosmosKeyCredential.java Outdated Show resolved Hide resolved
...microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/internal/AsyncDocumentClient.java Show resolved Hide resolved
.../microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/internal/TestConfigurations.java Show resolved Hide resolved
moderakh
moderakh approved these changes Aug 7, 2019
View reviewed changes
Copy link
Contributor

@moderakh moderakh left a comment

Choose a reason for hiding this comment

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

I have a minor comment on the implementation in the auth token provider, other than that and Chris' comment on public surface API, it looks good to me.

...zure-cosmos/src/main/java/com/azure/data/cosmos/internal/BaseAuthorizationTokenProvider.java Outdated Show resolved Hide resolved
srnagar
srnagar reviewed Aug 7, 2019
View reviewed changes
Copy link
Member

@srnagar srnagar left a comment

Choose a reason for hiding this comment

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

Without prior context on Cosmos APIs and client library guidelines, I have added a few comments based on the track 2 API guidelines documented here - https://azure.github.io/azure-sdk/java_design.html

If these don't apply to Cosmos, please ignore my comments.

sdk/cosmos/microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/CosmosClient.java Show resolved Hide resolved
sdk/cosmos/microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/CosmosClient.java Show resolved Hide resolved
sdk/cosmos/microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/CosmosKeyCredential.java Outdated Show resolved Hide resolved
sdk/cosmos/microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/CosmosKeyCredential.java Outdated Show resolved Hide resolved
...microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/internal/AsyncDocumentClient.java Show resolved Hide resolved
...zure-cosmos/src/main/java/com/azure/data/cosmos/internal/BaseAuthorizationTokenProvider.java Outdated Show resolved Hide resolved
...zure-cosmos/src/main/java/com/azure/data/cosmos/internal/BaseAuthorizationTokenProvider.java Show resolved Hide resolved
christopheranderson
christopheranderson approved these changes Aug 8, 2019
View reviewed changes
Copy link
Contributor

@christopheranderson christopheranderson left a comment

Choose a reason for hiding this comment

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

Approved with some nits on javadocs

sdk/cosmos/microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/CosmosKeyCredential.java Show resolved Hide resolved
sdk/cosmos/microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/CosmosKeyCredential.java Show resolved Hide resolved
sdk/cosmos/microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/CosmosKeyCredential.java Show resolved Hide resolved
alzimmermsft
alzimmermsft reviewed Aug 8, 2019
View reviewed changes
sdk/cosmos/microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/CosmosClientBuilder.java Show resolved Hide resolved
sdk/cosmos/microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/CosmosClientBuilder.java Show resolved Hide resolved
sdk/cosmos/microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/CosmosClient.java
@@ -136,7 +147,7 @@ TokenResolver getTokenResolver() {

/**
* CREATE a Database if it does not already exist on the service
* The {@link Mono} upon successful completion will contain a single cosmos database response with the
* The {@link Mono} upon successful completion will contain a single cosmos database response with the
Copy link
Member

Choose a reason for hiding this comment

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

Would this read better with the following:

Upon successful complete, a {@link Mono} containing the cosmos database response with the created or existing database will be returned

Potentially remove the statement given that its similar to the @return statement. Also, should cosmos be capitalized given it is the service's name?

...microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/internal/AsyncDocumentClient.java Show resolved Hide resolved
.../microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/internal/TestConfigurations.java
* If none of the above is set, emulator endpoint will be used.
*/
public final class TestConfigurations {
// REPLACE MASTER_KEY and HOST with values from your Azure Cosmos DB account.
// The default values are credentials of the local emulator, which are not used in any production environment.
// <!--[SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine")]-->
public static String MASTER_KEY =
System.getProperty("ACCOUNT_KEY",
System.getProperty("ACCOUNT_KEY",
StringUtils.defaultString(Strings.emptyToNull(
System.getenv().get("ACCOUNT_KEY")),
Copy link
Member

Choose a reason for hiding this comment

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

I know this is a testing file but should a different environment variable name be used to reduce chance of collision such as COSMOS_ACCOUNT_KEY?

.../microsoft-azure-cosmos/src/main/java/com/azure/data/cosmos/internal/TestConfigurations.java Show resolved Hide resolved
@kushagraThapar kushagraThapar merged commit 6e13b4a into Azure:master Aug 8, 2019
pull bot referenced this pull request in AzureSDKAutomation/azure-sdk-for-java Aug 8, 2019
@kushagraThapar @pull
Cosmos Key Credential (#4885)
dbb435f 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
pull bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 8, 2019
@kushagraThapar @pull
Cosmos Key Credential (Azure#4885)
d6e9e0e 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
pull bot pushed a commit to test-repo-tih/azure-sdk-for-java that referenced this pull request Aug 8, 2019
@kushagraThapar @pull
Cosmos Key Credential (Azure#4885)
5e02d98 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 14, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
619d3f2 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 14, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
0e1bf95 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 14, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
e93d0d0 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 14, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
acfeb7e 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 14, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
4086b31 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 14, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
11268b3 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 23, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
4bdb146 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 23, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
3144ccb 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 23, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
40dcfc5 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-bot-test bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 23, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
7573e9f 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-bot-test bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 23, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
e6b4e3a 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-bot-test bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 23, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
f9aa7bd 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-bot-test bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 23, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
662a684 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-bot-test bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 23, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
03e27fa 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-bot-test bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 23, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
7ba0757 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-bot-test bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 23, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
8732f83 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-bot-test bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 23, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
5c21072 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-bot-test bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 23, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
02ad61c 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-bot-test bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 23, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
ed75e70 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-bot-test bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 23, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
2a6026e 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-bot-test bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 23, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
6807612 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-bot-test bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 23, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
1a5d4b6 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-bot-test bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 23, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
96671c0 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 24, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
c72f587 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 24, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
dba5cee 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 24, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
5bfa8e2 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 24, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
250dbba 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 25, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
ec9a9e5 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 25, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
72543af 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 25, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
f18aaa7 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 25, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
e08a399 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 25, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
f4b0d6f 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 25, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
bf805ae 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 25, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
8263574 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 25, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
71e1577 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
openapi-sdkautomation bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 26, 2019
@kushagraThapar
Cosmos Key Credential (Azure#4885)
f4683ec 
Cosmos Key Credential holds the key credentials, and supports key rotations. User can update the key in CosmosKeyCredential object, and that will be reflected in SDK on the fly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srnagar srnagar srnagar left review comments

@alzimmermsft alzimmermsft alzimmermsft left review comments

@moderakh moderakh moderakh approved these changes

@g2vinay g2vinay g2vinay left review comments

@christopheranderson christopheranderson christopheranderson approved these changes

@danieljurek danieljurek Awaiting requested review from danieljurek

Assignees

@kushagraThapar kushagraThapar

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@kushagraThapar @christopheranderson @g2vinay @moderakh @alzimmermsft @srnagar