r/aws_kinesis_firehose_delivery_stream add http_endpoint_configuration

[ZsoltPath]

Community Note

• Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #14469

Release note for CHANGELOG:

resource/aws_kinesis_firehose_delivery_stream add http_endpoint_configuration 

[github-actions]

Thank you for your contribution! 🚀

Please note that typically Go dependency changes are handled in this repository by Renovate Bot or the maintainers. This is to prevent pull request merge conflicts and further delay reviews of contributions. Remove any changes to the go.mod, go.sum, and vendor/ files and commit them into this pull request.

Additional details:

• Check open pull requests with the dependencies label to view other dependency updates.
• If this pull request includes an update the AWS Go SDK (or any other dependency) version, only updates submitted via Renovate Bot will be merged. This pull request will need to remove these changes and will need to be rebased after the existing dependency update via Renovate Bot has been merged for this pull request to be reviewed.
• If this pull request is for supporting a new AWS service:
  • Ensure the new AWS service changes are following the Contributing Guide section on new services, in particular that the dependency addition and initial provider support are in a separate pull request from other changes (e.g. new resources). Contributions not following this item will not be reviewed until the changes are split.
  • If this pull request is already a separate pull request from the above item, you can ignore this message.

[aldegoeij]

@bflad any luck reviewing this yet 😇

[saintberry]

Also super keen to see this get merged. It will be much easier to manage New Relic integration using Firehose vs Lambda.

[bflad]

Hi @ZsoltPath 👋 Thank you for submitting this! At quick glance, this looks like its on the right track here for the code and documentation, however we will need to also ensure there is testing added to verify the functionality. Is this something you are able or interested to work on?

If so, for more information about how the testing works in this project, please see the Running and Writing Acceptance Testing section of the Contributing Guide. Copy-paste-modify of existing testing is fine and encouraged. The maintainers can try to answer any questions you might have.

If not, no big deal, either someone from the community can pick it up or eventually a maintainer.

Please let us know, thanks!

[ZsoltPath]

Hi @ZsoltPath 👋 Thank you for submitting this! At quick glance, this looks like its on the right track here for the code and documentation, however we will need to also ensure there is testing added to verify the functionality. Is this something you are able or interested to work on?

If so, for more information about how the testing works in this project, please see the Running and Writing Acceptance Testing section of the Contributing Guide. Copy-paste-modify of existing testing is fine and encouraged. The maintainers can try to answer any questions you might have.

If not, no big deal, either someone from the community can pick it up or eventually a maintainer.

Please let us know, thanks!

I'll give it a go.
I wasn't super keen on it because as I see very few of the resource's functions have tests already.

[ZsoltPath]

@bflad I'm stuck here with HCL formatting.
When I run terrafmt locally it passes without an issue:

terraform-provider-aws$ terrafmt diff --check --fmtcompat aws/resource_aws_kinesis_firehose_delivery_stream_test.go
terraform-provider-aws$ echo $?
0

Could you give me a hint what should I do? I just don't want to play around and push commits and commits to accidentally find the solution.

[bflad]

I'll need to ask around why terrafmt isn't working locally as I'm not able to reproduce the difference output myself, even though there definitely was whitespace issues. I tried to catch the ones I could see though and maybe this will square it away.

[ZsoltPath]

I'll need to ask around why terrafmt isn't working locally as I'm not able to reproduce the difference output myself, even though there definitely was whitespace issues. I tried to catch the ones I could see though and maybe this will square it away.

Yay! Thanks for finding these! I hate whitespace issues.

[bflad]

Hi @ZsoltPath 👋 Getting close. Are you able to run the acceptance testing? Please see the Running and Writing Acceptance Testing section of the Contributing Guide. The other resource tests look fine, just need to fix up this one. 👍

[ZsoltPath]

How the heck this new semgrep issue come up for aws/resource_aws_s3_bucket_public_access_block.go?
I haven't even opened that file :(

[bflad]

@ZsoltPath it is unrelated to your change and is due to merging #15925. It was fixed earlier today with #16068. semgrep is supposed to be better about ignoring unrelated changes in pull requests by comparing previous Git history, but seems like that isn't working correctly. Regardless, that issue will not affect me reviewing and potentially merging this if the acceptance testing is now working.

[bflad]

Thank you for your work on this, @ZsoltPath. 👍 It is close enough that I'm going to squash and merge this to fix up the pending test failure and ensure that retry_duration is properly configurable.

[ghost]

This has been released in version 3.15.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!