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

Bug: talhelper fails to parse talenv.yaml if the yaml document delimiter is present #220

Closed
mircea-pavel-anton opened this issue Nov 21, 2023 · 5 comments · Fixed by #231

Comments

@mircea-pavel-anton
Copy link
Collaborator

mircea-pavel-anton commented Nov 21, 2023

It seems that talhelper refuses to parse a talenv.yaml file if the yaml document delimiter is present:

mike@home-ops-devbox:/workspace/home-ops/infra/home-cluster$ ls -l
total 24
drwxr-xr-x 1 mike mike 4096 Nov 21 11:08 clusterconfig
drwxr-xr-x 1 mike mike 4096 Nov 19 18:38 patches
-rw-r--r-- 1 mike mike 3204 Nov 21 11:06 talconfig.yaml
-rw-r--r-- 1 mike mike   97 Nov 21 11:07 talenv.yaml
-rw-r--r-- 1 mike mike 8970 Nov 19 18:38 talsecret.sops.yaml
-rw-r--r-- 1 mike mike 4841 Nov 19 19:42 talsecret.yaml
mike@home-ops-devbox:/workspace/home-ops/infra/home-cluster$ cat talenv.yaml 
---
clusterName: home-ops
clusterEndpointIP: 10.0.10.10
clusterNetworkDomain: k8s.mirceanton.com
mike@home-ops-devbox:/workspace/home-ops/infra/home-cluster$ talhelper genconfig 
2023/11/21 11:09:28 failed to substitute env: variable ${clusterName} not set

But if I remove the --- at the top of the file:

mike@home-ops-devbox:/workspace/home-ops/infra/home-cluster$ cat talenv.yaml 
clusterName: home-ops
clusterEndpointIP: 10.0.10.10
clusterNetworkDomain: k8s.mirceanton.com
mike@home-ops-devbox:/workspace/home-ops/infra/home-cluster$ talhelper genconfig
generated config for nuc.home-ops.k8s.mirceanton.com in ./clusterconfig/home-ops-nuc.home-ops.k8s.mirceanton.com.yaml
generated config for srv.home-ops.k8s.mirceanton.com in ./clusterconfig/home-ops-srv.home-ops.k8s.mirceanton.com.yaml
generated config for minisforum.home-ops.k8s.mirceanton.com in ./clusterconfig/home-ops-minisforum.home-ops.k8s.mirceanton.com.yaml
generated client config in ./clusterconfig/talosconfig
generated .gitignore file in ./clusterconfig/.gitignore
mike@home-ops-devbox:/workspace/home-ops/infra/home-cluster$ 

This is not really a problem if the file is sops-encrypted, as sops automatically removes that delimiter, but it threw me off a bit initially.

@budimanjojo
Copy link
Owner

@mirceanton yeah I noticed about this too before, it's an issue of the dotenv library that I'm using and I had plan to report this issue upstream but forgot. I'll open an issue there tomorrow. Thanks for the report! 😁

@budimanjojo
Copy link
Owner

I have reported the bug to the library repo: joho/godotenv#217

@budimanjojo
Copy link
Owner

@mirceanton I don't think the upstream is gonna accept my PR since the library is not accepting PR (based on the pinned issue).

So I just fix it from here unless the upstream decided to fix this issue.

@mircea-pavel-anton
Copy link
Collaborator Author

mircea-pavel-anton commented Nov 24, 2023

@budimanjojo Yeah, it seems they don't want to accept any more features. I think there are 2 options here:

  • simply add a note in the documentation stating that, as a limitation of an upstream library, one should not use the delimiter in talenv files
  • read the file -> check for delimiter and remove it if it's there -> pass it to the parser

@budimanjojo
Copy link
Owner

@mirceanton The commit I pushed that closes this issue is the fix, so there should be no more problem with having yaml document delimiter in talenv.yaml 😉

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 a pull request may close this issue.

2 participants