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

providers/aws: Clarifies db_security_group usage docs. #2070

Merged
merged 2 commits into from
May 25, 2015
Merged

providers/aws: Clarifies db_security_group usage docs. #2070

merged 2 commits into from
May 25, 2015

Conversation

c4milo
Copy link
Contributor

@c4milo c4milo commented May 25, 2015

  • db_security_group is only intended to be used in EC2-Classic Platform.
    For DB instances in a VPC, we associate VPC security groups instead,
    when declaring the db_instance resource.
  • db_instance passwords are limited to 41 characters by AWS.

db_security_group is only intended to be used in EC2-Classic Platform.
For DB instances in a VPC, we associate VPC security groups instead,
when declaring the db_instance resource.
Provides an RDS security group resource.
Provides an RDS security group resource. This is only for DB instances in the
EC2-Classic Platform. For instances inside a VPC, use the
[`aws_db_instance.vpc_security_group_ids`](/docs/providers/aws/r/db_instance.html#vpc_security_group_ids)
Copy link
Member

Choose a reason for hiding this comment

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

👍 for links throughout docs.

@@ -45,7 +45,7 @@ The following arguments are supported:
* `name` - (Optional) The DB name to create. If omitted, no database is created
initially.
* `password` - (Required) Password for the master DB user. Note that this may
show up in logs, and it will be stored in the state file.
show up in logs, and it will be stored in the state file. Maximum 41 characters.
Copy link
Member

Choose a reason for hiding this comment

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

Keep in mind this is a resource covering all engines, therefore this statement is not always true according to the official docs, since not everyone may use MySQL:
http://docs.aws.amazon.com/AmazonRDS/latest/CommandLineReference/CLIReference-cmd-ModifyDBInstance.html

MySQL
Constraints: Must contain from 8 to 41 alphanumeric characters.

Oracle
Constraints: Must contain from 8 to 30 alphanumeric characters.

SQL Server
Constraints: Must contain from 8 to 128 alphanumeric characters.

PostgreSQL
Constraints: Must contain from 8 to 128 alphanumeric characters.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can just mention, that there are limitations and include the link above? Eventually if you can find a better link or better way of expressing this, I'm open for suggestions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. I'll add the link to the official AWS docs instead.

@c4milo
Copy link
Contributor Author

c4milo commented May 25, 2015

@radeksimko done.

@radeksimko
Copy link
Member

I think it doesn't have to look like warning nor it should be at the end of the page, but since those things are easy to fix, I will do it. 😉

For your own convenience in the future, it may be better to create PRs from a separate branch. It makes changes in-place much easier since you don't have to be worried about using git push --force in master branch and you can also create multiple independent PRs from multiple branches.

(sorry for explaining the obvious if you know all this and it was just a one-off accident 😃 )

radeksimko added a commit that referenced this pull request May 25, 2015
providers/aws: Clarifies db_security_group usage docs.
@radeksimko radeksimko merged commit d569e14 into hashicorp:master May 25, 2015
radeksimko added a commit to MeredithCorpOSS/terraform that referenced this pull request May 25, 2015
@c4milo
Copy link
Contributor Author

c4milo commented May 25, 2015

I think it doesn't have to look like warning nor it should be at the end of the page, but since those things are easy to fix, I will do it. 😉

Oh, I thought they were notes, not warnings. Thanks for fixing it for me anyways.

For your own convenience in the future, it may be better to create PRs from a separate branch. It makes changes in-place much easier since you don't have to be worried about using git push --force in master branch and you can also create multiple independent PRs from multiple branches.

Thanks for your suggestions! I don't mind being reminded about these things at all. These clarifications are also helpful for future contributors!

catsby added a commit that referenced this pull request May 28, 2015
…chment

* upstream/master: (21 commits)
  fix typo
  fix typo, use awslabs/aws-sdk-go
  Update CHANGELOG.md
  More internal links in template documentation.
  providers/aws: Requires ttl and records attributes if there isn't an ALIAS block.
  Condense switch fallthroughs into expr lists
  Fix docs for aws_route53_record params
  Update CHANGELOG.md
  provider/aws: Add IAM Server Certificate resource
  aws_db_instance docs updated per #2070
  providers/aws: Adds link to AWS docs about RDS parameters.
  Downgrade middleman to 3.3.12 as 3.3.13 does not exist
  providers/aws: Clarifies db_security_group usage.
  "More more" no more!
  Indentation issue
  Export ARN in SQS queue and SNS topic / subscription; updated tests for new AWS SDK errors; updated documentation.
  Changed Required: false to Optional: true in the SNS topic schema
  Initial SNS support
  correct resource name in example
  added attributes reference section for AWS_EBS_VOLUME
  ...
@ghost
Copy link

ghost commented May 2, 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 May 2, 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