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

WIP feat: allow cert-manager annotations on ingress based on environment variables PT.2 #112

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

cloud-j-luna
Copy link
Member

This PR allows cert-manager annotations on ingress based on environment variables.

@cloud-j-luna
Copy link
Member Author

Previously closed issue #86

cluster/util/environment.go Outdated Show resolved Hide resolved
cluster/kube/config.go Outdated Show resolved Hide resolved
operator/hostnameoperator/hostname_operator.go Outdated Show resolved Hide resolved
@troian
Copy link
Member

troian commented Apr 18, 2023

i'll summarize requests here so it would be easier to follow

  • we use viper as interface to parse and prevalidate configuration parameter that can be defined using:
    • default values
    • config file
    • env variables
    • flags
  • don't define env variables, but flags instead, each flag will have env variable automatically by Viper (refer to it's documentation if you are eager to know how it works). for example --home flag will have AKASH_HOME env variable bound to it.
  • the configuration must be evaluated and validated within command handler and all subsequent instantiation must use go types corresponding to its values. think it this way: type safety is a top priority
  • if value can take only two states then bool would be the correct type. for example
    instead
// sslEnable is a string type
if sslEnable != "" && sslEnable == "1" {

do

// sslEnable is a bool type
if sslEnable {

provider run would be a good place to start

@troian troian changed the title feat: allow cert-manager annotations on ingress based on environment variables PT.2 WIP feat: allow cert-manager annotations on ingress based on environment variables PT.2 Apr 20, 2023
@troian
Copy link
Member

troian commented Apr 20, 2023

i marked it as WIP until all issues are resolved. feel free to remove when done

@cloud-j-luna cloud-j-luna changed the title WIP feat: allow cert-manager annotations on ingress based on environment variables PT.2 feat: allow cert-manager annotations on ingress based on environment variables PT.2 Apr 21, 2023
@cloud-j-luna cloud-j-luna changed the title feat: allow cert-manager annotations on ingress based on environment variables PT.2 WIP feat: allow cert-manager annotations on ingress based on environment variables PT.2 Apr 23, 2023
@@ -21,7 +21,7 @@ kind-install-helm-chart-loki:
helm upgrade --install promtail grafana/promtail \
--version $(PROMTAIL_VERSION) \
--namespace loki-stack \
-f ../promtail-values.yaml
-f ../promtail-values.yamlk
Copy link
Contributor

Choose a reason for hiding this comment

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

should it be yamlk?

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.

3 participants