Added devbox #40

keyvaann · 2024-12-25T21:43:59Z

This PR adds Devbox to the project, it will facilitate the development with simplifying installing dependencies and configuration.
In the next PR Trivy and Checkov hooks will be activated to validate the configuration with them.

baixiac

Thanks for adding this. Left minor comments.

baixiac · 2024-12-30T12:56:29Z

cluster/variables.tf

-variable "environment" {
-  type        = string
-  description = "Environment name"
-  default     = "dev"
-}
-


Any particular reason why this was removed?

There was a linting error saying that this variable isn't in use.

I see. It is not used in "cluster" any more but only in "config".

Yes it has been not been removed from the config

baixiac · 2024-12-30T13:11:24Z

.pre-commit-config.yaml

+  # - id: terraform_trivy
+  #   args:
+  #   - --args=--severity CRITICAL
+  #   - --args=--skip-dirs="**/.terraform"
+  # - id: terraform_checkov
+  #   args:
+  #   - --args=--quiet
+  #   - --args=--skip-check CKV_AWS_23,CKV_AWS_355,CKV_AWS_290 # Temporarly skip these checks, Helm chart testing isn't needed for the repo


I saw this was mentioned to be enabled in the next step. Can you consider including this in the same PR?

Enabling this will return around 30 errors that should be fixed, some of them are easy to fix, the others I can't fix them and I'd need your help for them.
This PR is already rather big, it will be much bigger if we want to fix everything in on PR.

Excluding #37 and #38, here is the list of issues that checkov finds:

Check: CKV_TF_1: "Ensure Terraform module sources use a commit hash" Check: CKV_TF_1: "Ensure Terraform module sources use a commit hash" Check: CKV_TF_1: "Ensure Terraform module sources use a commit hash" Check: CKV_TF_1: "Ensure Terraform module sources use a commit hash" Check: CKV_TF_1: "Ensure Terraform module sources use a commit hash" Check: CKV_TF_1: "Ensure Terraform module sources use a commit hash" Check: CKV_TF_1: "Ensure Terraform module sources use a commit hash" Check: CKV_AWS_382: "Ensure no security groups allow egress from 0.0.0.0:0 to port -1" Check: CKV_TF_1: "Ensure Terraform module sources use a commit hash" Check: CKV_TF_1: "Ensure Terraform module sources use a commit hash" Check: CKV_TF_1: "Ensure Terraform module sources use a commit hash" Check: CKV_TF_1: "Ensure Terraform module sources use a commit hash" Check: CKV2_AWS_61: "Ensure that an S3 bucket has a lifecycle configuration" Check: CKV2_AWS_30: "Ensure Postgres RDS as aws_db_instance has Query Logging enabled" Check: CKV2_AWS_65: "Ensure access control lists for S3 buckets are disabled" Check: CKV_AWS_144: "Ensure that S3 bucket has cross-region replication enabled" Check: CKV2_AWS_39: "Ensure Domain Name System (DNS) query logging is enabled for Amazon Route 53 hosted zones" Check: CKV2_AWS_62: "Ensure S3 buckets should have event notifications enabled" Check: CKV2_AWS_6: "Ensure that S3 bucket has a Public Access block" Check: CKV2_AWS_60: "Ensure RDS instance with copy tags to snapshots is enabled" Check: CKV_AWS_21: "Ensure all data stored in the S3 bucket have versioning enabled" Check: CKV2_AWS_38: "Ensure Domain Name System Security Extensions (DNSSEC) signing is enabled for Amazon Route 53 public hosted zones" Check: CKV_AWS_18: "Ensure the S3 bucket has access logging enabled" Check: CKV_AWS_145: "Ensure that S3 buckets are encrypted with KMS by default" Check: CKV2_AWS_19: "Ensure that all EIP addresses allocated to a VPC are attached to EC2 instances" Check: CKV2_GHA_1: "Ensure top-level permissions are not set to write-all"

Alright. Will have a look and see if I can contribute, thinking 30 doesn't sound extremely large to me and maybe some of them can also be safely ignored (e.g., the rules about S3 are definitely not mandatory and should be applied case by case).

Indeed, some of them can be skipped.

Just tailored the checking rules. Both trivy and checkov are now enabled and PTAL.

… into devbox * 'main' of github.com:phidatalab/RADAR-K8s-Infrastructure: init before validate in make prepare

… into devbox

keyvaann · 2025-01-06T15:54:17Z

I fixed a few of the issues. But I think a few of skipped things should be enabled.

CKV2_AWS_38: DNSSEC signing needs to be optional.
- Why should this be optional?
CKV_AWS_145: Ensure that S3 buckets are encrypted with KMS by default.
- I think this should be enabled for all buckets by default as it is required for storing personal data in most regulations.
CKV_AWS_18: Ensure the S3 bucket has access logging enabled
- This is also something that is required by the regulation and it's good if we are able to set it up via Terraform.
CKV2_AWS_61: Ensure that an S3 bucket has a lifecycle configuration
- This can be handy to have to reduce costs. Especially for backup buckets.
CKV_AWS_21: Ensure all data stored in the S3 bucket have versioning enabled
- Sometimes there is a requirement for this one as well.
CKV_AWS_144: Ensure that S3 bucket has cross-region replication enabled
- This is also good to have to improve data availability.
CKV_AWS_23: Ensure every security group and rule has a description
- I think this is a good to have for documentation

keyvaann · 2025-01-06T16:13:46Z

Trivy is also reporting a few issues that I think would be useful to fix.

baixiac · 2025-01-06T18:02:01Z

Addressed some Trivy issues and would suggest focusing on HIGH and CRITICAL issues for this public template, as some users have already reported a low signal-to-noise ratio with Trivy/Checkov.

keyvaann · 2025-01-06T19:32:07Z

Addressed some Trivy issues and would suggest focusing on HIGH and CRITICAL issues for this public template, as some users have already reported a low signal-to-noise ratio with Trivy/Checkov.

We can do this for Trivy. I think for Checkov it's not possible unless you have an API key from their platofmr.

baixiac · 2025-01-07T10:13:41Z

I think for Checkov it's not possible unless you have an API key from their platofmr

True.

For these newly added SAST tools, my gut feeling is we need to strike a balance between cloud costs and risk mitigation. In private forks, users can customise the template however they wish.

keyvaann · 2025-01-07T10:25:19Z

Making a private fork and changing the templates will make it difficult to follow the upstream in future so I'm trying to have as much as possible in this repository to avoid a need for a private fork. If a risk mitigation solution is expensive it can be turned off by default but I think it's good to have the code for it here.

baixiac · 2025-01-07T11:47:35Z

AFAIK, not many ORGs publish their "live" IaC online. The risk is too high if malicious users spot vulnerabilities in their code or direct/transitive dependencies. SAST can only help to some extent so private forking followed by customisation is still a safer choice.

keyvaann · 2025-01-07T12:20:42Z

I see your point about the risks of publishing "live" IaC online, but since the whole RADAR-Base stack here is already open source, we’re not introducing new exposure by keeping these improvements in the main repository.

By including these features in the upstream—especially with riskier or cost-intensive options turned off by default—we ensure the repository remains a comprehensive and central resource. This reduces the need for private forks, which can make it harder for teams to keep up with upstream changes and updates over time.

While private forks do allow customization, they introduce fragmentation and maintenance overhead, which we can avoid by designing the public repo to be modular and adaptable. This approach aligns with open-source principles, encourages collaboration, and ensures any shared improvements benefit the broader community.

baixiac · 2025-01-07T13:42:49Z

Hmm... I never said having a baseline security is unnecessary nor was I questioning the open-source principles you wish to uphold. What I'm saying is this public template should be used as a general guide (seeding) and should never claim to be 100% secure for every possible use case, many of which are essentially unknown.

Disagreed on how much overhead a private forking will cause, given infrastructure updates occur much less frequently than application updates, and the former always requires careful review and even meticulous cherrypicking by DevOps/FinOps.

baixiac · 2025-01-08T12:15:19Z

The descriptions to the VPC endpoint SG and rules were added and PTAL @keyvaann.

keyvaann · 2025-01-08T16:06:25Z

I think we can merge this PR now. Remaining issues can be improved in a later stage.

baixiac

👍 👍

baixiac · 2025-01-08T17:32:02Z

Same thing happened before. Rerun the job can make it green but its worth finding out why it happens on the first run. cc @keyvaann

keyvaann · 2025-01-08T20:17:39Z

Same thing happened before. Rerun the job can make it green but its worth finding out why it happens on the first run. cc @keyvaann

The first failure looked like a network issue to me. The second one seems like something that can be investigated.

keyvaann requested a review from baixiac December 25, 2024 21:44

keyvaann force-pushed the devbox branch 2 times, most recently from 72e34cf to d08300a Compare December 25, 2024 22:29

Added devbox

1f8da4a

keyvaann force-pushed the devbox branch from d08300a to 1f8da4a Compare December 26, 2024 10:55

baixiac reviewed Dec 30, 2024

View reviewed changes

keyvaann and others added 2 commits December 30, 2024 16:56

Merge branch 'main' of github.com:phidatalab/RADAR-K8s-Infrastructure…

42801bd

… into devbox * 'main' of github.com:phidatalab/RADAR-K8s-Infrastructure: init before validate in make prepare

Merge branch 'main' of github.com:RADAR-base/RADAR-K8s-Infrastructure…

24e2c91

… into devbox

baixiac force-pushed the devbox branch 4 times, most recently from 4be00c2 to 8ccabb7 Compare January 3, 2025 16:44

configure and enable trivy and checkov

29f0dd2

baixiac force-pushed the devbox branch from 8ccabb7 to 29f0dd2 Compare January 3, 2025 17:42

keyvaann added 2 commits January 6, 2025 13:55

Ensure Terraform module sources use a commit hash

74bb056

Skip checkov checks in resource blocks instead of global skip

f0512b5

Ask Trivy to report MEDIUM and HIGH issues as well

476e83d

resolve issues reported by Trivy

97e4f4a

baixiac mentioned this pull request Jan 7, 2025

Add description to security groups #37

Closed

add descriptions to the VPC endpoint SG and rules

3bd276b

baixiac approved these changes Jan 8, 2025

View reviewed changes

keyvaann merged commit b8223c2 into main Jan 8, 2025
1 check passed

keyvaann deleted the devbox branch January 8, 2025 17:08

keyvaann restored the devbox branch January 8, 2025 21:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added devbox #40

Added devbox #40

keyvaann commented Dec 25, 2024

baixiac left a comment

baixiac Dec 30, 2024

keyvaann Dec 30, 2024

baixiac Dec 30, 2024

keyvaann Dec 30, 2024

baixiac Dec 30, 2024

keyvaann Dec 30, 2024 •

edited

Loading

keyvaann Dec 30, 2024

baixiac Dec 30, 2024

keyvaann Dec 30, 2024

baixiac Jan 3, 2025

keyvaann commented Jan 6, 2025

keyvaann commented Jan 6, 2025

baixiac commented Jan 6, 2025 •

edited

Loading

keyvaann commented Jan 6, 2025

baixiac commented Jan 7, 2025

keyvaann commented Jan 7, 2025

baixiac commented Jan 7, 2025

keyvaann commented Jan 7, 2025

baixiac commented Jan 7, 2025 •

edited

Loading

baixiac commented Jan 8, 2025

keyvaann commented Jan 8, 2025

baixiac left a comment

baixiac commented Jan 8, 2025

keyvaann commented Jan 8, 2025

Added devbox #40

Added devbox #40

Conversation

keyvaann commented Dec 25, 2024

baixiac left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keyvaann Dec 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keyvaann commented Jan 6, 2025

keyvaann commented Jan 6, 2025

baixiac commented Jan 6, 2025 • edited Loading

keyvaann commented Jan 6, 2025

baixiac commented Jan 7, 2025

keyvaann commented Jan 7, 2025

baixiac commented Jan 7, 2025

keyvaann commented Jan 7, 2025

baixiac commented Jan 7, 2025 • edited Loading

baixiac commented Jan 8, 2025

keyvaann commented Jan 8, 2025

baixiac left a comment

Choose a reason for hiding this comment

baixiac commented Jan 8, 2025

keyvaann commented Jan 8, 2025

keyvaann Dec 30, 2024 •

edited

Loading

baixiac commented Jan 6, 2025 •

edited

Loading

baixiac commented Jan 7, 2025 •

edited

Loading