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

provider/aws: Allow tags for kinesis streams #3397

Merged
merged 1 commit into from
Oct 12, 2015
Merged

provider/aws: Allow tags for kinesis streams #3397

merged 1 commit into from
Oct 12, 2015

Conversation

garrettheel
Copy link

Adds tags to kinesis streams. Tests pass locally and this shouldn't affect the tests that appear to have failed - not sure why this happened.

Related: #1296

log.Printf("[DEBUG] Creating tags: %#v", create)
var t map[string]*string
for _, tag := range create {
t[*tag.Key] = tag.Value
Copy link
Contributor

Choose a reason for hiding this comment

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

When I ran the acceptance tests, I got a crash here on setting the tags.

The var declaration above declares the storage for the map but it doesn't initialize it. I think you can fix this by changing the declaration to t := make(map[string]*string) .

2015/10/11 10:03:48 [DEBUG] Creating tags: []*kinesis.Tag{{
  Key: "Name",
  Value: "tf-test"
}}
panic: assignment to entry in nil map

goroutine 52 [running]:
github.com/hashicorp/terraform/builtin/providers/aws.setTagsKinesis(0xc20802cb60, 0xc208213440, 0x0, 0x0)
    src/github.com/hashicorp/terraform/builtin/providers/aws/tags_kinesis.go:45 +0x77b
github.com/hashicorp/terraform/builtin/providers/aws.resourceAwsKinesisStreamUpdate(0xc208213440, 0xbd36a0, 0xc2082a2240, 0x0, 0x0)
    src/github.com/hashicorp/terraform/builtin/providers/aws/resource_aws_kinesis_stream.go:88 +0xea
github.com/hashicorp/terraform/builtin/providers/aws.resourceAwsKinesisStreamCreate(0xc208213440, 0xbd36a0, 0xc2082a2240, 0x0, 0x0)
    src/github.com/hashicorp/terraform/builtin/providers/aws/resource_aws_kinesis_stream.go:81 +0x85d
github.com/hashicorp/terraform/helper/schema.(*Resource).Apply(0xc2080ae6c0, 0xc2080bf9e0, 0xc208259e80, 0xbd36a0, 0xc2082a2240, 0x6aa601, 0x0, 0x0)
    src/github.com/hashicorp/terraform/helper/schema/resource.go:145 +0x2c1
github.com/hashicorp/terraform/helper/schema.(*Provider).Apply(0xc2080bf080, 0xc2081368c0, 0xc2080bf9e0, 0xc208259e80, 0x1, 0x0, 0x0)
    src/github.com/hashicorp/terraform/helper/schema/provider.go:162 +0x1da
github.com/hashicorp/terraform/terraform.(*EvalApply).Eval(0xc208136a80, 0x7fc773201960, 0xc2082de9c0, 0x0, 0x0, 0x0, 0x0)
    src/github.com/hashicorp/terraform/terraform/eval_apply.go:67 +0x62a
github.com/hashicorp/terraform/terraform.EvalRaw(0x7fc773203328, 0xc208136a80, 0x7fc773201960, 0xc2082de9c0, 0x0, 0x0, 0x0, 0x0)
    src/github.com/hashicorp/terraform/terraform/eval.go:53 +0x254
github.com/hashicorp/terraform/terraform.(*EvalSequence).Eval(0xc20829b9e0, 0x7fc773201960, 0xc2082de9c0, 0x0, 0x0, 0x0, 0x0)
    src/github.com/hashicorp/terraform/terraform/eval_sequence.go:10 +0xf9
github.com/hashicorp/terraform/terraform.EvalRaw(0x7fc773201b28, 0xc20829b9e0, 0x7fc773201960, 0xc2082de9c0, 0x0, 0x0, 0x0, 0x0)
    src/github.com/hashicorp/terraform/terraform/eval.go:53 +0x254
github.com/hashicorp/terraform/terraform.(*EvalOpFilter).Eval(0xc2080e32f0, 0x7fc773201960, 0xc2082de9c0, 0x0, 0x0, 0x0, 0x0)
    src/github.com/hashicorp/terraform/terraform/eval_filter_operation.go:37 +0x71
github.com/hashicorp/terraform/terraform.EvalRaw(0x7fc773201b50, 0xc2080e32f0, 0x7fc773201960, 0xc2082de9c0, 0x0, 0x0, 0x0, 0x0)
    src/github.com/hashicorp/terraform/terraform/eval.go:53 +0x254
github.com/hashicorp/terraform/terraform.(*EvalSequence).Eval(0xc20829a940, 0x7fc773201960, 0xc2082de9c0, 0x0, 0x0, 0x0, 0x0)
    src/github.com/hashicorp/terraform/terraform/eval_sequence.go:10 +0xf9
github.com/hashicorp/terraform/terraform.EvalRaw(0x7fc773201b28, 0xc20829a940, 0x7fc773201960, 0xc2082de9c0, 0x0, 0x0, 0x0, 0x0)
    src/github.com/hashicorp/terraform/terraform/eval.go:53 +0x254
github.com/hashicorp/terraform/terraform.Eval(0x7fc773201b28, 0xc20829a940, 0x7fc773201960, 0xc2082de9c0, 0x0, 0x0, 0x0, 0x0)
    src/github.com/hashicorp/terraform/terraform/eval.go:34 +0x79
github.com/hashicorp/terraform/terraform.func·027(0xfc84c0, 0xc2080e24e0, 0x0, 0x0)
    src/github.com/hashicorp/terraform/terraform/graph.go:198 +0x906
github.com/hashicorp/terraform/dag.func·006(0xfc84c0, 0xc2080e24e0, 0xc2082123c0, 0xc208212420)
    src/github.com/hashicorp/terraform/dag/dag.go:238 +0x106
created by github.com/hashicorp/terraform/dag.(*AcyclicGraph).Walk
    src/github.com/hashicorp/terraform/dag/dag.go:247 +0x877

@apparentlymart
Copy link
Contributor

Thanks for the work here, @garrettheel!

Other than the little bug I mentioned inline, this seems to work just fine. If you'd like to write up some docs for it then that'd make this easier to merge 😀 ; If you look in the website dir in the root of this repo you can see how you can get a local copy of the website up using Vagrant.

@garrettheel
Copy link
Author

Thanks for picking up that bug @apparentlymart! I've since tested more throughly to make sure this works properly.

Bug is fixed and docs have been updated

@apparentlymart apparentlymart merged commit 36f78cc into hashicorp:master Oct 12, 2015
apparentlymart added a commit that referenced this pull request Oct 12, 2015
@apparentlymart
Copy link
Contributor

Thanks! 🚢

@radeksimko radeksimko mentioned this pull request Nov 21, 2015
7 tasks
@ghost
Copy link

ghost commented Apr 30, 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 30, 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.

3 participants