Skip to content

Commit

Permalink
Fix response code, closes #219 (#221)
Browse files Browse the repository at this point in the history
* skip SSH key when https:// git URL is used

* disable proxy environment variables if NO_PROXY is set

* fix comparison of response code, fixes #219

* fix TestNoProxy

* remove deprecation
  • Loading branch information
xorpaul authored Jan 25, 2024
1 parent 1a4a266 commit 8cae529
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 17 deletions.
12 changes: 6 additions & 6 deletions forge.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ func queryForgeAPI(fm ForgeModule) ForgeResult {
mutex.Unlock()
defer resp.Body.Close()

if strings.TrimSpace(resp.Status) == "200 OK" {
if resp.StatusCode == http.StatusOK {
// need to get latest version
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
Expand All @@ -218,10 +218,10 @@ func queryForgeAPI(fm ForgeModule) ForgeResult {

return ForgeResult{true, fr.versionNumber, fr.md5sum, fr.fileSize}

} else if strings.TrimSpace(resp.Status) == "304 Not Modified" {
} else if resp.StatusCode == http.StatusNotModified {
Debugf("Got 304 nothing to do for module " + fm.author + "-" + fm.name)
return ForgeResult{false, "", "", 0}
} else if strings.TrimSpace(resp.Status) == "404 Not Found" {
} else if resp.StatusCode == http.StatusNotFound {
Fatalf("Received 404 from Forge for module " + fm.author + "-" + fm.name + " using URL " + url + " Does the module really exist and is it correctly named?")
return ForgeResult{false, "", "", 0}
}
Expand Down Expand Up @@ -305,7 +305,7 @@ func getMetadataForgeModule(fm ForgeModule) ForgeModule {
}
defer resp.Body.Close()

if strings.TrimSpace(resp.Status) == "200 OK" {
if resp.StatusCode == http.StatusOK {
body, err := ioutil.ReadAll(resp.Body)

if err != nil {
Expand Down Expand Up @@ -390,7 +390,7 @@ func downloadForgeModule(name string, version string, fm ForgeModule, retryCount
}
defer resp.Body.Close()

if strings.TrimSpace(resp.Status) == "200 OK" {
if resp.StatusCode == http.StatusOK {
wgForgeModule.Add(1)
go func() {
defer wgForgeModule.Done()
Expand Down Expand Up @@ -424,7 +424,7 @@ func downloadForgeModule(name string, version string, fm ForgeModule, retryCount
Fatalf("Error while writing to MultiWriter " + err.Error())
}
}()
} else if strings.TrimSpace(resp.Status) == "404 Not Found" {
} else if resp.StatusCode == http.StatusNotFound {
Fatalf("Received 404 from Forge using URL " + url +
"\nCheck if the module name '" + fm.author + "-" + fm.name + "' and version '" + version + "' really exist" +
"\nUsed in Puppet environment '" + fm.sourceBranch + "'")
Expand Down
2 changes: 1 addition & 1 deletion g10k_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3254,7 +3254,7 @@ func TestNoProxy(t *testing.T) {
// fmt.Println(string(out))

expectedLines := []string{
"found matching NO_PROXY URL, trying to disable http_proxy for git clone --mirror https://localgit.domain.tld/foo/bar.git /tmp/g10k/modules/https-__localgit.domain.tld_foo_bar.git",
"found matching NO_PROXY URL, trying to disable http_proxy and https_proxy env variables for git clone --mirror https://localgit.domain.tld/foo/bar.git /tmp/g10k/modules/https-__localgit.domain.tld_foo_bar.git",
}
for _, expectedLine := range expectedLines {
if !strings.Contains(string(out), expectedLine) {
Expand Down
2 changes: 1 addition & 1 deletion git.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func doMirrorOrUpdate(gitModule GitModule, workDir string, retryCount int) bool
isInModulesCacheDir := strings.HasPrefix(workDir, config.ModulesCacheDir)

explicitlyLoadSSHKey := true
if len(gitModule.privateKey) == 0 || strings.Contains(gitModule.git, "github.com") || gitModule.useSSHAgent {
if len(gitModule.privateKey) == 0 || strings.Contains(gitModule.git, "github.com") || gitModule.useSSHAgent || strings.HasPrefix(gitModule.git, "https://") {
if gitModule.useSSHAgent || len(gitModule.privateKey) == 0 {
explicitlyLoadSSHKey = false
} else if isControlRepo {
Expand Down
20 changes: 14 additions & 6 deletions helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,18 @@ func executeCommand(command string, commandDir string, timeout int, allowFail bo
execCommand.Dir = commandDir
}
if disableHttpProxy {
Debugf("found matching NO_PROXY URL, trying to disable http_proxy for " + command)
execCommand.Env = append(os.Environ(), "http_proxy=")
execCommand.Env = append(os.Environ(), "https_proxy=")
Debugf("found matching NO_PROXY URL, trying to disable http_proxy and https_proxy env variables for " + command)
// execCommand.Env = append(os.Environ(), "http_proxy=")
// execCommand.Env = append(os.Environ(), "https_proxy=")
os.Unsetenv("http_proxy")
os.Unsetenv("https_proxy")
os.Unsetenv("HTTP_PROXY")
os.Unsetenv("HTTPS_PROXY")
execCommand.Env = os.Environ()
Debugf("exec OS env:" + strings.Join(execCommand.Env, ",") + " for command " + command)
} else {
execCommand.Env = os.Environ()
Debugf("exec OS env:" + strings.Join(execCommand.Env, ",") + " for command " + command)
}
out, err := execCommand.CombinedOutput()
duration := time.Since(before).Seconds()
Expand Down Expand Up @@ -350,9 +359,8 @@ func matchGitRemoteURLNoProxy(url string) bool {
noProxy := os.Getenv("NO_PROXY")
for _, np := range strings.Split(noProxy, ",") {
if len(np) > 0 {
Debugf("found NO_PROXY setting: " + np)
if strings.Contains(url, np) {
// fmt.Println("found matching", np, "for", url)
Debugf("found NO_PROXY setting: " + np + " matching " + url)
return true
}
}
Expand All @@ -362,7 +370,7 @@ func matchGitRemoteURLNoProxy(url string) bool {
for _, np := range strings.Split(noProxyL, ",") {
if len(np) > 0 {
if strings.Contains(url, np) {
// fmt.Println("found matching", np, "for", url)
Debugf("found no_proxy setting: " + np + " matching " + url)
return true
}
}
Expand Down
5 changes: 2 additions & 3 deletions puppetfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package main

import (
"fmt"
"io/ioutil"
"os"
"path/filepath"
"regexp"
Expand Down Expand Up @@ -317,10 +316,10 @@ func resolvePuppetfile(allPuppetfiles map[string]Puppetfile) {

for _, moduleDir := range pf.moduleDirs {
moduleDir = normalizeDir(filepath.Join(pf.workDir, moduleDir))
exisitingModuleDirsFI, _ := ioutil.ReadDir(moduleDir)
exisitingModuleDirsFI, _ := os.ReadDir(moduleDir)
mutex.Lock()
for _, exisitingModuleDir := range exisitingModuleDirsFI {
//fmt.Println("adding dir: ", moduleDir+exisitingModuleDir.Name())
// fmt.Println("adding dir: ", filepath.Join(moduleDir, exisitingModuleDir.Name()))
exisitingModuleDirs[filepath.Join(moduleDir, exisitingModuleDir.Name())] = empty
}
mutex.Unlock()
Expand Down

0 comments on commit 8cae529

Please sign in to comment.