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

feat: Add validation on argocd_applications app name #1513

Merged
merged 6 commits into from
Mar 29, 2023

Conversation

catmand00
Copy link
Contributor

@catmand00 catmand00 commented Mar 27, 2023

‼️ PLEASE READ THIS FIRST ‼️

The direction for EKS Blueprints will soon shift from providing an all-encompassing, monolithic "framework" and instead focus more on how users can organize a set of modular components to create the desired solution on Amazon EKS. We have updated the examples to show how we use the https://github.com/terraform-aws-modules/terraform-aws-eks for EKS cluster and node group creation. We will not be accepting any PRs that apply to EKS cluster or node group creation process. Any such PR may be closed by the maintainers.

We are hitting also the pause button on new add-on creations at this time until a future roadmap for add-ons is finalized. Please do not submit new add-on PRs. Any such PR may be closed by the maintainers.

Please track progress, learn what's new and how the migration path would look like to upgrade your current Terraform deployments. We welcome the EKS Blueprints community to continue the discussion in issue #1421

What does this PR do?

Add validation to the var.applications variable used by argocd submodule, which uses the var.argocd_applications variable from the root module. The allowed name of the key is restricted by the same restrictions on kubernetes pod names, since the key is used for the name for the Application object, and the Application uses its name as the prefix of the resulting pod.
🛑 Please open an issue first to discuss any significant work and flesh out details/direction - we would hate for your time to be wasted.
Consult the CONTRIBUTING guide for submitting pull-requests.

Motivation

More

  • Yes, I have tested the PR using my local account setup (Provide any test evidence report under Additional Notes)
  • Yes, I have updated the docs for this feature
  • Yes, I ran pre-commit run -a with this PR

For Moderators

  • E2E Test successfully complete before merge?

Additional Notes

I have tested this locally using the same config that resulted in an error (linked in the related Issue), and I receive the error during the plan phase instead (as intended). Changing the name to test-app passes the validation (also intended).

@catmand00 catmand00 changed the title feat: add validation on argocd_applications app name feat: Add validation on argocd_applications app name Mar 27, 2023
Copy link
Contributor

@bryantbiggs bryantbiggs left a comment

Choose a reason for hiding this comment

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

Thank you!

@bryantbiggs
Copy link
Contributor

@catmand00 would you mind opening the same PR in our new addons repo https://github.com/aws-ia/terraform-aws-eks-blueprints-addons when you get a chance - thank you!

@bryantbiggs bryantbiggs marked this pull request as ready for review March 29, 2023 18:16
@bryantbiggs bryantbiggs requested a review from a team as a code owner March 29, 2023 18:16
@bryantbiggs bryantbiggs merged commit aaba020 into aws-ia:main Mar 29, 2023
gminiba pushed a commit to gminiba/terraform-aws-eks-blueprints that referenced this pull request Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggestion: validate the argocd_application keys early
3 participants