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: Adding some other simple S3 Bucket Object (Optional) Inputs #3265

Merged
merged 6 commits into from
Oct 12, 2015

Conversation

stack72
Copy link
Contributor

@stack72 stack72 commented Sep 17, 2015

Added the following to the S3 Bucket Object:

  • cache_control
  • content_disposition
  • content_encoding
  • content_language
  • content_type

Tests pass as expected:

make testacc TEST=./builtin/providers/aws TESTARGS='-run=S3BucketObject' 2>~/tf.log
go generate ./...
TF_ACC=1 go test ./builtin/providers/aws -v -run=S3BucketObject -timeout 90m
=== RUN TestAccAWSS3BucketObject_basic
--- PASS: TestAccAWSS3BucketObject_basic (4.64s)
=== RUN TestAccAWSS3BucketObject_withContentCharacteristics
--- PASS: TestAccAWSS3BucketObject_withContentCharacteristics (3.45s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    8.109s

@stack72 stack72 changed the title Adding some other simple S3 Bucket Object (Optional) Inputs provider/aws: Adding some other simple S3 Bucket Object (Optional) Inputs Sep 28, 2015
@stack72
Copy link
Contributor Author

stack72 commented Oct 8, 2015

@catsby while you are on a merging spree, this one should be a pretty simple addition :) It just adds support for some more pieces of the S3 puzzle

* `content_disposition` - (Optional) Specifies presentational information for the object.
* `content_encoding` - (Optional) Specifies what content encodings have been applied to the object and thus what decoding mechanisms must be applied to obtain the media-type referenced by the Content-Type header field.
* `content_language` - (Optional) The language the content is in.
* `content_type` - (Optional) A standard MIME type describing the format of the object data.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we throw in a few examples of these? e.g. for cache_control, two examples would be no-cache, or max-age=3000? Is the max-age=3000 even valid? Some guidance would help people (like myself...) on how to use it correctly

…ecking for an empty string in the new S3 bucket object params
@stack72
Copy link
Contributor Author

stack72 commented Oct 8, 2015

@catsby thanks for reviewing this. I have added links to the correct sections of w3c docs for some of the more complex documentation (e.g. cache_control and content_disposition) and then added some samples for content_type and content_language

Also changed the PutObject input to check for empty strings before adding to the struct

@stack72
Copy link
Contributor Author

stack72 commented Oct 8, 2015

@catsby ok, I have added some explanations here and changed to support the empty string checks

FYI, master had removed the Update method and 'Forced New' on everything. So I was getting this error:

--- FAIL: TestProvider-2 (0.00s)
    provider_test.go:24: err: aws_s3_bucket_object: No Update defined, must set ForceNew on: []string{"cache_control", "content_disposition", "content_encoding", "content_language", "content_type"}

By making the same changes to my code, the world was a better place :)

I think this may be ready for you now

"content_type": &schema.Schema{
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like at least this one needs to be computed as well, I'm getting a plan loop with this config:

provider "aws" {
  region = "us-west-2"
}

resource "aws_s3_bucket" "object_bucket" {
    bucket = "tf-object-test-bucket-1234"
}

resource "aws_s3_bucket_object" "object" {
    bucket = "${aws_s3_bucket.object_bucket.bucket}"
    key = "test-key"
    source = "terraform.tfstate"
}
~ aws_s3_bucket_object.object
    content_type: "binary/octet-stream" => ""

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@catsby can a param be both Optional and Computed?

@catsby
Copy link
Contributor

catsby commented Oct 8, 2015

One final bit and we're good to go

…f whether you set a content-type, AWS will always set a content-type
@stack72
Copy link
Contributor Author

stack72 commented Oct 9, 2015

@catsby we are good with this now :)

@catsby
Copy link
Contributor

catsby commented Oct 12, 2015

Thanks @stack72 !

catsby added a commit that referenced this pull request Oct 12, 2015
provider/aws: Adding some other simple S3 Bucket Object (Optional) Inputs
@catsby catsby merged commit 8152f58 into hashicorp:master Oct 12, 2015
@stack72 stack72 deleted the aws-s3-bucket-update branch October 12, 2015 19:24
@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.

2 participants