Use CABundle when fetching chart with helm.chart field of fleet.yaml file
#4185
Conversation
p-se
force-pushed
the
SURE-9881
branch
7 times, most recently
from
81d1690 to
c63d1b6
Compare
p-se
force-pushed
the
SURE-9881
branch
2 times, most recently
from
c7ede99 to
d343974
Compare
| HTTPSPort = 4343 | ||
| ) | ||
|
|
||
| type gitRepoTestValues struct { |
There was a problem hiding this comment.
By using this struct instead of inline structs, the additional fields added in gitrepo.yaml don't need to be provided. The empty value preserves the previous behavior and it is a bit clearer which values can be provided.
| // replace replaces string s with r in the file located at path. That file must exist and be writable. | ||
| func replace(path string, s string, r string) { | ||
| b, err := os.ReadFile(path) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
|
|
||
| b = bytes.ReplaceAll(b, []byte(s), []byte(r)) | ||
|
|
||
| err = os.WriteFile(path, b, 0644) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| } | ||
|
|
||
| // getGitRepoStatus retrieves the status of the gitrepo with the provided name. | ||
| func getGitRepoStatus(g Gomega, k kubectl.Command, name string) fleet.GitRepoStatus { | ||
| gr, err := k.Get("gitrepo", name, "-o=json") | ||
|
|
||
| g.Expect(err).ToNot(HaveOccurred()) | ||
|
|
||
| var gitrepo fleet.GitRepo | ||
| _ = json.Unmarshal([]byte(gr), &gitrepo) | ||
|
|
||
| return gitrepo.Status | ||
| } | ||
|
|
There was a problem hiding this comment.
Those are already provided in the singlecluster_test package.
| return nil, err | ||
| } | ||
| _, err = g.Update(repo, UpdateForce) | ||
|
|
There was a problem hiding this comment.
I'm reusing the functionality of Update in Create and the newly added Add method. Create can be used a second time to force push an update to an already existing repo.
| if auth.CABundle != nil { | ||
| tmpFile, err := os.CreateTemp("", "ca-bundle") | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer os.Remove(tmpFile.Name()) | ||
| if _, err := tmpFile.Write(auth.CABundle); err != nil { | ||
| return nil, err | ||
| } | ||
| if err := os.Setenv("GIT_SSL_CAINFO", tmpFile.Name()); err != nil { | ||
| return nil, err | ||
| } | ||
| defer os.Unsetenv("GIT_SSL_CAINFO") | ||
| } | ||
|
|
There was a problem hiding this comment.
I'm setting the data here because the logic to determine which CABundle and value for SkipInsecureTLSVerify is supposed to be used is determined in the gitjob controller. The alternative is to map all secrets into the pod of the Job and have it handled here more or less again. The Gitjob controllers needs to do it anyway because we apparently need those values in different places. Those are passed as arguments, environment variables and volumes to the pod. So, I thought it would be simpler to keep the logic there.
There was a problem hiding this comment.
I wonder if this logic wouldn't find a better home in apply.go's addAuthToOpts, where fleet apply deals with populating that Auth.CABundle field.
Instead of creating an additional file, perhaps this logic could then directly read the CACertsFile 🤔
Happy to discuss.
There was a problem hiding this comment.
Long story short, the CABundle is replaced per path if helmSecretNameForPaths is configured. That happens after apply.go's addAuthToOpts.
| if auth.CABundle != nil { | ||
| tmpFile, err := os.CreateTemp("", "ca-bundle") | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer os.Remove(tmpFile.Name()) | ||
| if _, err := tmpFile.Write(auth.CABundle); err != nil { | ||
| return nil, err | ||
| } | ||
| if err := os.Setenv("GIT_SSL_CAINFO", tmpFile.Name()); err != nil { | ||
| return nil, err | ||
| } | ||
| defer os.Unsetenv("GIT_SSL_CAINFO") | ||
| } | ||
|
|
There was a problem hiding this comment.
I wonder if this logic wouldn't find a better home in apply.go's addAuthToOpts, where fleet apply deals with populating that Auth.CABundle field.
Instead of creating an additional file, perhaps this logic could then directly read the CACertsFile 🤔
Happy to discuss.
p-se
force-pushed
the
SURE-9881
branch
4 times, most recently
from
41ed1c9 to
26b02a0
Compare
|
Incidental that all tests pass in the first run when I cherry picked the fix for the race? 🤔 Edit: Twice. |
3 participants
Refers to #3646
Additional Information
Checklist
fleet-docs repository.