Skip to content

Commit 82e9a0c

Browse files
authored
improvement: changes in PR and Issue templates, CONTRIBUTING.md (#132)
* improvement: Moved code conventions into CONTRIBUTING.md #131 * improvement: updated PR and Issue templates #131
1 parent fe887c3 commit 82e9a0c

File tree

4 files changed

+353
-319
lines changed

4 files changed

+353
-319
lines changed

.github/CONTRIBUTING.md

Lines changed: 305 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,32 @@ email, or any other method with the owners of this repository before making a ch
55

66
Please note we have a code of conduct, please follow it in all your interactions with the project.
77

8+
## Table of contents
9+
10+
- [Contributing](#contributing)
11+
- [Table of contents](#table-of-contents)
12+
- [Pull Request Process](#pull-request-process)
13+
- [Checklists for contributions](#checklists-for-contributions)
14+
- [Semantic Pull Requests](#semantic-pull-requests)
15+
- [Coding conventions](#coding-conventions)
16+
- [Names and approaches used in code](#names-and-approaches-used-in-code)
17+
- [Base project name](#base-project-name)
18+
- [Unique prefix of resource names](#unique-prefix-of-resource-names)
19+
- [Separators](#separators)
20+
- [Depends_on](#depends_on)
21+
- [Resource names](#resource-names)
22+
- [Variable names](#variable-names)
23+
- [Output names](#output-names)
24+
- [Names of terraform files, directories, and modules](#names-of-terraform-files-directories-and-modules)
25+
- [General configuration files](#general-configuration-files)
26+
- [Specific configuration files](#specific-configuration-files)
27+
- [Modules](#modules)
28+
- [Project structure](#project-structure)
29+
830
## Pull Request Process
931

10-
1. Ensure any install or build dependencies are removed before the end of the layer when doing a build.
11-
2. Update the README.md with details of changes to the interface, this includes new environment variables, exposed ports, useful file locations, and container parameters.
32+
1. Ensure any install or build dependencies are removed before the end of the layer when doing a build.
33+
2. Update the README.md with details of changes to the interface, this includes new environment variables, exposed ports, useful file locations, and container parameters.
1234
3. Once all outstanding comments and checklist items have been addressed, your contribution will be merged! Merged PRs will be included in the next release. The aws-eks-base maintainers take care of updating the CHANGELOG as they merge.
1335

1436
## Checklists for contributions
@@ -31,3 +53,284 @@ To generate changelog, Pull Requests or Commits must have semantic and must foll
3153

3254
The `chore` prefix skipped during changelog generation. It can be used for `chore: update changelog` commit message by example.
3355

56+
## Coding conventions
57+
58+
This section contains the most basic recommendations for users and contributors on coding, naming, etc. The goal is consistent, standardized, readable code. Additions, suggestions and changes are welcome.
59+
60+
### Names and approaches used in code
61+
62+
#### Base project name
63+
64+
The base name is set in the name variable in `variables.tf` and is used to form unique resource names:
65+
66+
```
67+
variable "name" {
68+
default = "demo"
69+
}
70+
```
71+
72+
#### Unique prefix of resource names
73+
74+
Based on the variables `name`, `region` and the `terraform.workspace` value, we form a unique prefix for resource names:
75+
76+
```
77+
locals {
78+
env = terraform.workspace == "default" ? var.environment : terraform.workspace
79+
short_region = var.short_region[var.region]
80+
name = "${var.name}-${local.env}-${local.short_region}"
81+
}
82+
```
83+
84+
Prefix example:
85+
86+
- name = "demo"
87+
- region = "us-east-2"
88+
- terraform.workspace = "test"
89+
90+
`demo-test-use2`
91+
92+
The `local.name` value is then used as a prefix for all `name` and `name_prefix` attributes. This allows us to run copies of the infrastructure even in one account.
93+
94+
#### Separators
95+
96+
- For the `name` or `name_prefix` attributes of resources, modules, etc., as well as for output data values, the hyphen character `-` is used as the separator:
97+
98+
```
99+
name = "${local.name}-example"
100+
```
101+
102+
or
103+
104+
```
105+
name = "demo-test-use2-example"
106+
```
107+
108+
- For complex names in the declaration of resources, variables, modules, and outputs in code, the underscore character `_` is used:
109+
110+
```
111+
resource "aws_iam_role_policy_attachment" "pritunl_server"{
112+
}
113+
114+
variable "cluster_name" {
115+
}
116+
117+
module "security_groups" {
118+
}
119+
```
120+
121+
> Use `name_prefix` where possible
122+
123+
#### Depends_on
124+
125+
When you need to add `depends_on` to a resource or a module you should put it at the end of the block with empty line in front of it.
126+
127+
```
128+
resource "aws_eks_addon" "coredns" {
129+
...
130+
addon_version = var.addon_coredns_version
131+
132+
depends_on = [module.eks]
133+
}
134+
```
135+
136+
#### Resource names
137+
138+
- The resource type should not be duplicated in the resource name (either partially or in full):
139+
- Good: `resource "aws_route_table" "public" {}`
140+
- Bad: `resource "aws_route_table" "public_route_table" {}`
141+
- Bad: `resource "aws_route_table" "public_aws_route_table" {}`
142+
143+
- If the resource is unique within the module, you should use `this` when naming. For example, the module contains one `aws_nat_gateway` resource and several `aws_route_table` resources; in this case, `aws_nat_gateway` should be named `this`, while `aws_route_table` should have more meaningful names, e.g. `private`, `public`, `database`:
144+
145+
```
146+
resource "aws_nat_gateway" "this" {
147+
...
148+
}
149+
resource "aws_route_table" "public"{
150+
...
151+
}
152+
resource "aws_route_table" "private"{
153+
...
154+
}
155+
```
156+
157+
- Nouns must be used for names
158+
159+
#### Variable names
160+
161+
- Use the same variable names, description, and default value as defined in the official terraform documentation for the resource you are working on
162+
- Don’t specify `type = "list"` if there is `default = []`
163+
- Don’t specify `type = "map"` if there is `default = {}`
164+
- Use plurals in the names of variables like list and map:
165+
166+
```
167+
variable "rds_parameters" {
168+
default = [
169+
{
170+
name = "log_min_duration_statement"
171+
value = "2000"
172+
},
173+
]
174+
}
175+
```
176+
177+
- Always use description for variables
178+
- The higher the level of variable declaration, the more desirable it is to use semantic prefixes for each variable:
179+
180+
```
181+
variable "ecs_instance_type" {
182+
...
183+
}
184+
185+
variable "rds_instance_type" {
186+
...
187+
}
188+
```
189+
190+
#### Output names
191+
192+
- Output names must be understandable outside terraforms and outside the module’s context (when a user uses the module, the type and attribute of the return value must be clear)
193+
- The general recommendation for data output naming is that the name should describe the value inside and should not have redundancies
194+
- The correct structure for output names looks like `{name}_{type}_{attribute}` for non-unique attributes and resources and `{type}_{attribute}` for unique ones; an example of displaying one of several security groups and a unique public address:
195+
196+
```
197+
output "alb_security_group_id" {
198+
description = "The ID of the example security group"
199+
value = "${aws_security_group.alb.id}"
200+
}
201+
202+
output "public_ip" {
203+
description = "Public Ip Address of the Elasti IP assigned to ec2 instance"
204+
value = "${aws_eip.this.public_ip}"
205+
}
206+
```
207+
208+
- If the return value is a list, it must have a plural name
209+
- Use description for outputs
210+
211+
### Names of terraform files, directories, and modules
212+
213+
#### General configuration files
214+
215+
Each terraform module and configuration contains a set of general files ending in `.tf`:
216+
217+
- `main.tf` - contains terraform settings if it is the top layer; or the main working code if it is a module
218+
- `variables.tf` - module input values
219+
- `outputs.tf` - module output values
220+
221+
Besides these, there may be:
222+
223+
- `locals.tf` - contains a set of variables obtained by interpolation from remote state, outputs, variables, etc
224+
- `providers.tf` - contains settings from terraform providers, e.g. `aws`, `kubernetes`, etc
225+
- `iam.tf` - IAM configurations of policies, roles, etc
226+
227+
This is not a full list; each configuration, module, or layer may need additional files and manifests. The objective is to name them as succinctly and closer in meaning to the content as possible. Do not use prefixes.
228+
229+
> Terraform itself doesn't care how many files you create. It collects all layer and module manifests into one object, builds dependencies, and executes.
230+
231+
#### Specific configuration files
232+
233+
These configuration files and manifests include the following: `data "template_file"` or `templatefile ()` template resources, a logical resource group placed in a separate `.tf` file, one or more deployments to k8s using `resource "helm_release"`, module initialization, aws resources that do not require a separate module, etc.
234+
235+
> It should be noted that since some kind of a logical group of resources is being, why not move it all into a separate module. But it turned out that it is easier to manage helm releases, templates for them, and additional resources in separate `.tf` files at the root of a layer. And for many such configurations, when moving to modules, the amount of code can double + what we move to modules is usually what we are going to reuse.
236+
237+
Each specific `.tf` file must begin with a prefix indicating the service or provider to which the main resource or group being created belongs, e.g. `aws`. Optionally, the type of service is indicated next, e.g. `iam`. Next comes the name of the main service or resource or resource group declared inside, and after that, an explanatory suffix can optionally be added if there are several such files. All the parts of the name are separated by hyphens`
238+
239+
So the formula looks like this: `provider|servicename`-[`optional resource/service type`]-`main resourcename|group-name`-[`optional suffix`].tf
240+
241+
Examples:
242+
243+
- `aws-vpc.tf` - terraform manifest describing the creation of a single vpc
244+
- `aws-vpc-stage.tf` - terraform manifest describing the creation of one of several vpc, for staging
245+
- `eks-namespaces.tf` - group of namespaces created in the EKS cluster
246+
- `eks-external-dns.tf` - contains the description of external-dns service deployment to the EKS cluster
247+
- `aws-ec2-pritunl.tf` - contains the initialization of the module that creates an EC2 instance in AWS with pritunl configured
248+
249+
#### Modules
250+
251+
The approach to naming module directories is exactly the same as for specific `.tf` files and uses this formula: `provider|servicename`-[`optional resource/service type`]-`main resourcename|group-name`-[`optional suffix`]
252+
253+
Examples:
254+
255+
- `eks-rbac-ci` - module for creating rbac for CI inside the EKS cluster
256+
- `aws-iam-autoscaler` - module for creating IAM policies for autoscaler
257+
- `aws-ec2-pritunl` - module for creating pritunl ec2 instance
258+
259+
### Project structure
260+
261+
```
262+
aws-eks-base
263+
┣ docker
264+
┣ examples
265+
┣ helm-charts
266+
┣ terraform
267+
┃ ┣ layer1-aws
268+
┃ ┃ ┣ examples
269+
┃ ┃ ┣ templates
270+
┃ ┃ ┣ aws-acm.tf
271+
┃ ┃ ┣ aws-eks.tf
272+
┃ ┃ ┣ aws-vpc.tf
273+
┃ ┃ ┣ locals.tf
274+
┃ ┃ ┣ main.tf
275+
┃ ┃ ┣ outputs.tf
276+
┃ ┃ ┣ providers.tf
277+
┃ ┃ ┗ variables.tf
278+
┃ ┣ layer2-k8s
279+
┃ ┃ ┣ examples
280+
┃ ┃ ┣ templates
281+
┃ ┃ ┣ eks-aws-node-termination-handler.tf
282+
┃ ┃ ┣ eks-cert-manager.tf
283+
┃ ┃ ┣ eks-certificate.tf
284+
┃ ┃ ┣ eks-cluster-autoscaler.tf
285+
┃ ┃ ┣ eks-cluster-issuer.tf
286+
┃ ┃ ┣ eks-external-dns.tf
287+
┃ ┃ ┣ eks-external-secrets.tf
288+
┃ ┃ ┣ eks-namespaces.tf
289+
┃ ┃ ┣ eks-network-policy.tf
290+
┃ ┃ ┣ eks-nginx-ingress-controller.tf
291+
┃ ┃ ┣ locals.tf
292+
┃ ┃ ┣ main.tf
293+
┃ ┃ ┣ outputs.tf
294+
┃ ┃ ┣ providers.tf
295+
┃ ┃ ┣ ssm-ps-secrets.tf
296+
┃ ┃ ┗ variables.tf
297+
┃ ┗ modules
298+
┃ ┃ ┣ aws-iam-alb-ingress-controller
299+
┃ ┃ ┣ aws-iam-autoscaler
300+
┃ ┃ ┣ aws-iam-ci
301+
┃ ┃ ┣ aws-iam-external-dns
302+
┃ ┃ ┣ aws-iam-grafana
303+
┃ ┃ ┣ aws-iam-roles
304+
┃ ┃ ┣ aws-iam-s3
305+
┃ ┃ ┣ aws-iam-ssm
306+
┃ ┃ ┣ eks-rbac-ci
307+
┃ ┃ ┣ kubernetes-namespace
308+
┃ ┃ ┣ kubernetes-network-policy-namespace
309+
┃ ┃ ┣ pritunl
310+
┃ ┃ ┗ self-signed-certificate
311+
┣ .editorconfig
312+
┣ .gitignore
313+
┣ .gitlab-ci.yml
314+
┣ .pre-commit-config.yaml
315+
┣ README.md
316+
┗ README_OLD.md
317+
```
318+
319+
---
320+
321+
| FILE / DIRECTORY| DESCRIPTION |
322+
| --------------- |:-------------:|
323+
| docker/ | custom dockerfiles for examples |
324+
| examples/ | example k8s deployments |
325+
| helm-charts/ | directory contains custom helm charts |
326+
| helm-charts/certificate | helm chart which creates ssl certificate for nginx ingress |
327+
| helm-charts/cluster-issuer | helm chart which creates cluster-issuer using cert manager cdrs |
328+
| helm-charts/elk | umbrella chart to deploy elk stack |
329+
| helm-charts/teamcity | helm chart which deploys teamcity agent and/or server |
330+
|terraform/| directory contains terraform configuration files |
331+
|terraform/layer1-aws| directory contains aws resources |
332+
|terraform/layer2-k8s| directory contains resources deployed to k8s-EKS |
333+
|terraform/modules| directory contains terraform modules |
334+
|.editorconfig| |
335+
|.gitlab-ci.yml||
336+
|.pre-commit-config.yaml||

.github/ISSUE_TEMPLATE.md

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,35 @@
1-
# I have issues
1+
# Prerequisites
22

3-
## I'm submitting a...
3+
Please answer the following questions for yourself before submitting an issue. **YOU MAY DELETE THE PREREQUISITES SECTION.**
44

5-
* [ ] bug report
6-
* [ ] feature request
7-
* [ ] improvement request
5+
- [ ] I am running the latest version
6+
- [ ] I checked the documentation and found no answer
7+
- [ ] I checked to make sure that this issue has not already been filed
88

9-
## What is the current behavior?
9+
# Expected Behavior
1010

11+
Please describe the behavior you are expecting
1112

13+
# Current Behavior
1214

13-
## If this is a bug, how to reproduce? Please include a code sample if relevant.
15+
What is the current behavior?
1416

17+
# Failure Information (for bugs)
1518

19+
Please help provide information about the failure if this is a bug. If it is not a bug, please remove the rest of this template.
1620

17-
## What's the expected behavior?
21+
## Steps to Reproduce
1822

23+
Please provide detailed steps for reproducing the issue.
1924

25+
1. step 1
26+
2. step 2
27+
3. you get it...
2028

21-
## Are you able to fix this problem and submit a PR? Link here if you have already.
22-
23-
## Environment details
29+
## Context
2430

2531
* Affected module version:
2632
* OS:
2733
* Terraform version:
2834

29-
## Any other relevant info
35+
## Any other relevant info including logs

.github/PULL_REQUEST_TEMPLATE.md

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,21 @@
1-
# PR o'clock
2-
3-
## Description
1+
# PR Description
42

53
Please explain the changes you made here and link to any relevant issues.
64

7-
### Checklist
5+
Fixes # (issue)
6+
7+
## Type of change
8+
9+
Delete options that are not relevant.
10+
11+
- [ ] Bug fix (non-breaking change which fixes an issue)
12+
- [ ] New feature (non-breaking change which adds functionality)
13+
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
14+
- [ ] Documentation update
15+
16+
# Checklist
817

9-
- [ ] Update the README.md with details of changes to the interface, this includes new environment variables, exposed ports, useful file locations, and container parameters.
18+
- [ ] My code follows the style guidelines of this project
19+
- [ ] I have performed a self-review of my own code
20+
- [ ] I have commented my code, particularly in hard-to-understand areas
21+
- [ ] I have made corresponding changes to the documentation

0 commit comments

Comments
 (0)