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

API docs: Add docs to Terraform backends #1268

Open
ansgarm opened this issue Nov 8, 2021 · 3 comments
Open

API docs: Add docs to Terraform backends #1268

ansgarm opened this issue Nov 8, 2021 · 3 comments
Labels
cdktf documentation Improvements or additions to documentation good first issue Good for newcomers priority/important-longterm Medium priority, to be worked on within the following 1-2 business quarters. size/medium estimated < 1 week

Comments

@ansgarm
Copy link
Member

ansgarm commented Nov 8, 2021

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue 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 issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

There are currently no docstrings on the constructs we have for Terraform backends.

See this one for example: https://github.com/hashicorp/terraform-cdk/blob/main/packages/cdktf/lib/backends/s3-backend.ts

It should at least include a link to https://www.terraform.io/docs/language/settings/backends/s3.html

References

@ansgarm ansgarm added documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers cdktf needs-priority Issue has not yet been prioritized; this will prompt team review labels Nov 8, 2021
@DanielMSchmidt DanielMSchmidt added priority/important-soon High priority, to be worked on as part of our current release or the following one. size/medium estimated < 1 week and removed needs-priority Issue has not yet been prioritized; this will prompt team review labels Dec 2, 2021
@DanielMSchmidt DanielMSchmidt added this to the 0.10 (tentative) milestone Dec 2, 2021
@schersh schersh added priority/important-longterm Medium priority, to be worked on within the following 1-2 business quarters. and removed priority/important-soon High priority, to be worked on as part of our current release or the following one. labels Jan 25, 2022
@schersh schersh removed this from the 0.10 (tentative) milestone Jan 25, 2022
@aayushharwani-aidash
Copy link
Contributor

@ansgarm I am interested in working on this. Can you plz guide ?

@ansgarm
Copy link
Member Author

ansgarm commented May 5, 2022

Hi @aayushharwani-aidash 👋 Wow! That's great!

I'll use the S3 backend as an example, but feel free to pick your favorite(s) – you also don't have to do all at once to get a PR accepted and merged 🙂

Our backends can be found here and the docs on them are here.

So let's take the S3Backend and the related docs as an example.

The backend has a S3BackendProps type that defines the arguments that you can set on the backend:

export interface S3BackendProps {
readonly bucket: string;
readonly key: string;
readonly region?: string;
readonly endpoint?: string;

The docs contain descriptions for those config keys. For example for the region property:

region - (Required) AWS Region of the S3 Bucket and DynamoDB Table (if used). This can also be sourced from the AWS_DEFAULT_REGION and AWS_REGION environment variables.
(...)

A docstring for this property could look like this:

export interface S3BackendProps {
  // ...
  
  /**
   * AWS Region of the S3 Bucket and DynamoDB Table (if used). This can also
   * be sourced from the AWS_DEFAULT_REGION and AWS_REGION environment 
   * variables.
   */
  readonly region?: string;
  // ...

Note: The docs state that the region is required to be set, but in our code region is optional ("?") because it does not need to be set via the S3BackendProps config, but could also be set via an environment variable.

Besides adding docstrings to the properties, it is also useful to add a general docstring to the S3Backend class itself:

export class S3Backend extends TerraformBackend {
constructor(scope: Construct, private readonly props: S3BackendProps) {
super(scope, "backend", "s3");
}

The introductory section of the backend docs seems to fit quite well. And adding a link to the docs allows the user to access more information if required.

/**
 * Stores the state as a given key in a given bucket on Amazon S3. This backend
 * also supports state locking and consistency checking via Dynamo DB, which
 * can be enabled by setting the dynamodb_table field to an existing DynamoDB
 * table name. A single DynamoDB table can be used to lock multiple remote
 * state files. Terraform generates key names that include the values of the
 * bucket and key variables.
 * 
 * Warning! It is highly recommended that you enable Bucket Versioning on the
 * S3 bucket to allow for state recovery in the case of accidental deletions
 * and human error.
 * 
 * Read more about this backend in the Terraform docs:
 * https://www.terraform.io/language/settings/backends/s3
 */
export class S3Backend extends TerraformBackend {
  constructor(scope: Construct, private readonly props: S3BackendProps) {
    super(scope, "backend", "s3");
  }
  // ...

If there's anything left unclear, feel free to ask! I hope this explains what we had in mind here.

@aayushharwani-aidash
Copy link
Contributor

Thanks @ansgarm , I got your point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cdktf documentation Improvements or additions to documentation good first issue Good for newcomers priority/important-longterm Medium priority, to be worked on within the following 1-2 business quarters. size/medium estimated < 1 week
Projects
None yet
Development

No branches or pull requests

5 participants