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 Terraform module for dex-auth #228

Merged
merged 6 commits into from
Sep 24, 2024

Conversation

DnPlas
Copy link
Contributor

@DnPlas DnPlas commented Sep 18, 2024

This commit adds the terraform/ directory to the root of the repository to host the Terraform module of this charm. This follows the standard set in CC006. For more information please also refer to canonical/argo-operators/pull/198.

Part of #226

Testing instructions

  1. Check the correctness of the module with tox -e tflint
  2. Apply with terraform apply -var "channel=latest/edge" -var "model_name=kubeflow" --auto-approve
  3. Wait for the charm to be deployed.

For reviewers

This module must expose all the information in the metadata.yaml of this charm, make sure you review it by comparing that file to the Tf files in this PR.

This commit adds the terraform/ directory to the root of the repository to host
the Terraform module of this charm. This follows the standard set in CC006.
For more information please also refer to canonical/argo-operators/pull/198.

Fixes #226
Copy link
Contributor

@orfeas-k orfeas-k left a comment

Choose a reason for hiding this comment

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

Looking good, left some comments.

terraform/outputs.tf Show resolved Hide resolved
mvlassis
mvlassis previously approved these changes Sep 20, 2024
Copy link

@mvlassis mvlassis left a comment

Choose a reason for hiding this comment

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

LGTM. Note that there is an issue with the CI regarding the model name which we should handle.

The latest standard for the terraform/ README.md is that the compatibility
note can be obviated since the module at the branch is compatible with the
charm in the same branch.

Part of #266
Copy link
Contributor

@orfeas-k orfeas-k left a comment

Choose a reason for hiding this comment

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

Approved but

  1. Let's see what the CI does
  2. Let's not forget to backport this.

@DnPlas DnPlas merged commit b923697 into main Sep 24, 2024
10 checks passed
@DnPlas DnPlas deleted the KF-6162-dex-auth-terraform-module branch September 24, 2024 11:55
DnPlas added a commit that referenced this pull request Sep 24, 2024
* feat: add Terraform module for dex-auth

This commit adds the terraform/ directory to the root of the repository to host
the Terraform module of this charm. This follows the standard set in CC006.
For more information please also refer to canonical/argo-operators/pull/198.

Part of #226
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants