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

SNS applications #3335

Closed
wants to merge 3 commits into from
Closed

SNS applications #3335

wants to merge 3 commits into from

Conversation

dalehamel
Copy link

This adds support for SNS applications. This allows managing iOS, Android, and other mobile application push notifications via SNS in terraform, which is a use case we have.

This is modeled heavily after SNS topics.

The biggest obvious issue with this is that the private key will be stored in terraform state. A potential work-around for this would be to hash the contents store the hash instead of the actual contents in some way, but I'm not sure on how to do that.

Tested basic things like create/update/destroy manually.

Some things I'll expect would be needed to get this merged:

  • Don't store secrets in state file
  • Documentation
  • Acceptance tests

@phinze @catsby for review

cc @thegedge

@dalehamel
Copy link
Author

resource "aws_sns_application" "testapp" {
  name = "my-new-testapp"
  platform = "APNS_SANDBOX"
  credential = "${file("mykey.key")}"
  principal = "${file("mycert.pem")}"
}

Sample invokation

@dalehamel dalehamel changed the title WIP for SNS applications SNS applications Sep 28, 2015
@dalehamel
Copy link
Author

@radeksimko @phinze @catsby ok, should be ready for a review / don't consider this a WIP anymore.

I've moved the secret (credential) to use StateFunc, so that we only store a hash. I still store the principal in plaintext, but that's either a cert or a client id and shouldn't be considered secret anyways.

This is working for me, so if the code looks good I can write the docs up.

Let me know if acceptance tests are needed to get these accepted (I might have a hard time writing them, as I've never done this for terraform yet).

@radeksimko
Copy link
Member

Thanks @dalehamel for submitting this.

Acceptance test, at least a basic one for resource existence, would definitely help to move this PR forward.

Feel free to get some inspiration from aws_sns_topic which has actually quite a simple test: https://github.com/hashicorp/terraform/blob/master/builtin/providers/aws/resource_aws_sns_topic_test.go

@dalehamel
Copy link
Author

@radeksimko cool - i'll try and whip something up. The tricky thing here is that we'll need a pair of credentials to try and authenticate with... hopefuly they don't have to be valid! I'll try and just create some mock x509 cert/keys.

@apparentlymart apparentlymart added wip and removed wip labels Oct 4, 2015
@djeiejxjdjc
Copy link

Really think I'm tapped here . sns sam client in my logcat extreme . other weird stuff. Anyone have good suggestions for a brother ?

@catsby
Copy link
Contributor

catsby commented Jan 8, 2016

Hey @dalehamel – I'm going to close this for now, let us know if you plan on picking it back up sometime

@catsby catsby closed this Jan 8, 2016
@megaqube
Copy link

megaqube commented Aug 12, 2016

@dalehamel I may have a need for something like this soon. Would you mind if I picked it up where you left off? Thanks!

@dalehamel
Copy link
Author

Please do

On Fri, Aug 12, 2016 at 11:35 AM, Xirel notifications@github.com wrote:

@dalehamel https://github.com/dalehamel I may have a need for someone
like this soon. Would you mind if I picked it up where you left off? Thanks!


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3335 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAlwd3QZonrOtVeLS5KPgBGqbJJgivWFks5qfJLGgaJpZM4GEoBI
.

@megaqube
Copy link

@radeksimko I have unit tests for this written up, and they pass with my GCM Api Key. But before I submit this, I'm wondering what the suggestion is for not submitting my own GCM Api Key. The tests will not pass without a valid key. Thanks!

@megaqube
Copy link

Pull Request Submitted, build failing as expecting without the GCM and APNS keys in the environment variable. Looking for ways to overcome this without providing our credentials.

https://travis-ci.org/hashicorp/terraform/builds/153362909

@MattBlack85
Copy link

any news on this?

@megaqube
Copy link

@MattBlack85 The implementation was done a few months back. Tests, Documentation, everything should be up to HashiCorp standards. Please take a look at my pull request here: #8294

A couple of folks have been asking for it over in that Pull Request as well, please join in if it's something you want merged in as well. Thanks!

@ghost
Copy link

ghost commented Apr 21, 2020

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants