Skip to content

fix(elasticsearch): Domain.fromDomainAttributes gives "Invalid URL" when endpoint is a token#15219

Merged
mergify[bot] merged 3 commits intoaws:masterfrom
HelioStrike:domainattributes
Jun 22, 2021
Merged

fix(elasticsearch): Domain.fromDomainAttributes gives "Invalid URL" when endpoint is a token#15219
mergify[bot] merged 3 commits intoaws:masterfrom
HelioStrike:domainattributes

Conversation

@HelioStrike
Copy link
Copy Markdown
Contributor

@HelioStrike HelioStrike commented Jun 21, 2021

Domain.fromDomainAttributes throws the error "Invalid URL" when a token endpoint is provided. In this PR, the domain name is retrieved from the scope using the arn present in DomainAttributes, and if no name is found, we try to get it from the endpoint (as it has been done so far). Also added a test to verify that the error disappears with this change, even when it is given token attributes.

closes #15188


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@BenChaimberg BenChaimberg self-assigned this Jun 21, 2021
@BenChaimberg
Copy link
Copy Markdown
Contributor

Please add a summary description of the changes contained in this PR

Copy link
Copy Markdown
Contributor

@BenChaimberg BenChaimberg left a comment

Choose a reason for hiding this comment

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

the modes of these files were changed to allow execution; please revert

@gitpod-io
Copy link
Copy Markdown

gitpod-io bot commented Jun 22, 2021

@mergify mergify bot dismissed BenChaimberg’s stale review June 22, 2021 04:24

Pull request has been modified.

@HelioStrike
Copy link
Copy Markdown
Contributor Author

@BenChaimberg Updated the PR. Had to change modes due to some permission issue with yarn. Reverted it now, and also modified tests to check equality.
Also, when modifying a PR, should I force push modified commits, or add a commit on top?

@BenChaimberg
Copy link
Copy Markdown
Contributor

Please commit on top, as Github doesn't have great support for diffing between force-pushed commits

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 22, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 1086315
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit ecb5af8 into aws:master Jun 22, 2021
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 22, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

Seiya6329 pushed a commit to Seiya6329/aws-cdk that referenced this pull request Jun 22, 2021
…hen endpoint is a token (aws#15219)

`Domain.fromDomainAttributes` throws the error "Invalid URL" when a token endpoint is provided. In this PR, the domain name is retrieved from the scope using the arn present in `DomainAttributes`, and if no name is found, we try to get it from the endpoint (as it has been done so far). Also added a test to verify that the error disappears with this change, even when it is given token attributes.

closes aws#15188

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
matthewsvu pushed a commit to matthewsvu/aws-cdk that referenced this pull request Jun 22, 2021
…hen endpoint is a token (aws#15219)

`Domain.fromDomainAttributes` throws the error "Invalid URL" when a token endpoint is provided. In this PR, the domain name is retrieved from the scope using the arn present in `DomainAttributes`, and if no name is found, we try to get it from the endpoint (as it has been done so far). Also added a test to verify that the error disappears with this change, even when it is given token attributes.

closes aws#15188

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this pull request Aug 26, 2021
…hen endpoint is a token (aws#15219)

`Domain.fromDomainAttributes` throws the error "Invalid URL" when a token endpoint is provided. In this PR, the domain name is retrieved from the scope using the arn present in `DomainAttributes`, and if no name is found, we try to get it from the endpoint (as it has been done so far). Also added a test to verify that the error disappears with this change, even when it is given token attributes.

closes aws#15188

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
mergify bot pushed a commit that referenced this pull request Jul 5, 2022
Documentation for changes made in PR #15219 - issue #15188

Normally one would not put the domain endpoint as a static value in a CF stack.
A more realistic use case is to manage an opensearch domain from CF stack A and use it via CF stack B, C, D ...
Stack A would export the domainEndpoint and also domainArn - the other stacks can then import those values and import the domain.

This PR will add an example for the realistic scenario so people do not end up with errors like "Invalid URL" when a token endpoint is provided.



----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
daschaa pushed a commit to daschaa/aws-cdk that referenced this pull request Jul 9, 2022
Documentation for changes made in PR aws#15219 - issue aws#15188

Normally one would not put the domain endpoint as a static value in a CF stack.
A more realistic use case is to manage an opensearch domain from CF stack A and use it via CF stack B, C, D ...
Stack A would export the domainEndpoint and also domainArn - the other stacks can then import those values and import the domain.

This PR will add an example for the realistic scenario so people do not end up with errors like "Invalid URL" when a token endpoint is provided.



----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
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.

(elasticsearch): Domain.fromDomainAttributes gives "Invalid URL" when endpoint is a token

3 participants