Set git SSL information only for gitlab host
What does this MR do?
This MR overrides git SSL config only for the gitlab host
Why was this MR needed?
See #2148 (closed)
Are there points in the code the reviewer needs to double check?
Does this MR meet the acceptance criteria?
-
Documentation created/updated - Tests
-
Added for this feature/bug -
All builds are passing
-
-
Branch has no merge conflicts with master
(if you do - rebase it please)
What are the relevant issue numbers?
Closes #2148 (closed)
Merge request reports
Activity
- Resolved by Alessio Caiazza
Test can be seen at https://gitlab.com/nolith/test-runner-2148/-/jobs/32762518
Cloning into '/nolith/test-runner-2148'... * Couldn't find host gitlab.com in the .netrc file; using defaults * Trying 52.167.219.168... * TCP_NODELAY set * Connected to gitlab.com (52.167.219.168) port 443 (#0) * ALPN, offering http/1.1 * Cipher selection: ALL:!EXPORT:!EXPORT40:!EXPORT56:!aNULL:!LOW:!RC4:@STRENGTH * successfully set certificate verify locations: * CAfile: /nolith/test-runner-2148.tmp/CI_SERVER_TLS_CA_FILE CApath: none
and submodule
Submodule 'vendor/github.com/tevino/abool' (https://github.com/tevino/abool.git) registered for path 'vendor/github.com/tevino/abool' Cloning into '/nolith/test-runner-2148/vendor/github.com/tevino/abool'... * Couldn't find host github.com in the .netrc file; using defaults * Trying 192.30.253.112... * TCP_NODELAY set * Connected to github.com (192.30.253.112) port 443 (#0) * ALPN, offering http/1.1 * Cipher selection: ALL:!EXPORT:!EXPORT40:!EXPORT56:!aNULL:!LOW:!RC4:@STRENGTH * successfully set certificate verify locations: * CAfile: /etc/ssl/certs/ca-certificates.crt CApath: none
assigned to @ayufan
- Resolved by Alessio Caiazza
- Resolved by Alessio Caiazza
- Resolved by Alessio Caiazza
assigned to @nolith
@ayufan the following patch is the cleanest approch I found without changing git global configuration.
The problem is that
git submodules
ignores local config and only uses the global one. So if you are using submodules from your self-signed gitlab instance the build will not work.commit c028737b59877cb54b90b35c330c7a8e05d0adaa (HEAD -> refs/heads/2148-fix-ssl-cert) Author: Alessio Caiazza <acaiazza@gitlab.com> Date: Mon Sep 18 17:11:02 2017 +0200 Do not touch git global config diff --git a/shells/abstract.go b/shells/abstract.go index 2791d4a8..d6f1184c 100644 --- a/shells/abstract.go +++ b/shells/abstract.go @@ -30,18 +30,20 @@ func (b *AbstractShell) writeExports(w ShellWriter, info common.ShellScriptInfo) } } -func (b *AbstractShell) writeGitSSLConfig(w ShellWriter, info common.ShellScriptInfo) error { - repoURL, err := url.Parse(info.Build.GitInfo.RepoURL) +func (b *AbstractShell) gitSSLConfig(w ShellWriter, build *common.Build) (conf map[string]string) { + repoURL, err := url.Parse(build.GitInfo.RepoURL) if err != nil { - return err + w.Warning("git SSL config: Can't parse repository URL. %s", err) + return } repoURL.Path = "" repoURL.RawPath = "" repoURL.User = nil gitlabHost := repoURL.String() + conf = make(map[string]string) - for _, variable := range info.Build.GetCITLSVariables() { + for _, variable := range build.GetCITLSVariables() { var configKey, configValue string switch variable.Key { case "CI_SERVER_TLS_CA_FILE": @@ -55,18 +57,19 @@ func (b *AbstractShell) writeGitSSLConfig(w ShellWriter, info common.ShellScript } configValue = w.TmpFile(variable.Key) composedConfigKey := fmt.Sprintf("http.%s.%s", gitlabHost, configKey) - w.Command("git", "config", "--global", composedConfigKey, configValue) + conf[composedConfigKey] = configValue } - return nil + return } func (b *AbstractShell) writeCloneCmd(w ShellWriter, build *common.Build, projectDir string) { - templateDir := w.MkTmpDir("git-template") - args := []string{"clone", "--no-checkout", build.GitInfo.RepoURL, projectDir, "--template", templateDir} + args := []string{"clone", "--no-checkout", build.GitInfo.RepoURL, projectDir, "--config", "fetch.recurseSubmodules=false"} + for key, value := range b.gitSSLConfig(w, build) { + args = append(args, "--config", fmt.Sprintf("%s=%s", key, value)) + } w.RmDir(projectDir) - w.Command("git", "config", "-f", path.Join(templateDir, "config"), "fetch.recurseSubmodules", "false") if depth := build.GetGitDepth(); depth != "" { w.Notice("Cloning repository for %s with git depth set to %s...", build.GitInfo.Ref, depth) @@ -91,6 +94,10 @@ func (b *AbstractShell) writeFetchCmd(w ShellWriter, build *common.Build, projec w.Cd(projectDir) w.Command("git", "config", "fetch.recurseSubmodules", "false") + for key, value := range b.gitSSLConfig(w, build) { + w.Command("git", "config", key, value) + } + // Remove .git/{index,shallow}.lock files from .git, which can fail the fetch command // The file can be left if previous build was terminated during git operation w.RmFile(".git/index.lock") @@ -330,15 +337,12 @@ func (b *AbstractShell) writeSubmoduleUpdateCmds(w ShellWriter, info common.Shel func (b *AbstractShell) writeGetSourcesScript(w ShellWriter, info common.ShellScriptInfo) (err error) { b.writeExports(w, info) - if err := b.writeGitSSLConfig(w, info); err != nil { - return err - } if info.PreCloneScript != "" && info.Build.GetGitStrategy() != common.GitNone { b.writeCommands(w, info.PreCloneScript) } - if err = b.writeCloneFetchCmds(w, info); err != nil { + if err := b.writeCloneFetchCmds(w, info); err != nil { return err }
Next step
I think we should inform the builder if it's running inside a container (disposable environment) or not. In the first case change the global config and go on, on the second case change only local settings and documents how to configure your enviroment properly.
mentioned in merge request !690 (merged)
I start to agree that this is the only solution :)
Edited by Kamil Trzcińskiadded Deliverable ~1672339 typebug customer labels
added 35 commits
-
0b725834...a2980917 - 33 commits from branch
master
- 8fa4ecf8 - Set git SSL information only for gitlab host
- d60067d7 - Do not touch git global config
-
0b725834...a2980917 - 33 commits from branch
@ayufan I've rebased after !690 (merged) work.
assigned to @ayufan
- Resolved by Alessio Caiazza
- Resolved by Alessio Caiazza
- Resolved by Alessio Caiazza
assigned to @nolith
Waiting for the pipelines.
assigned to @ayufan
- Resolved by Alessio Caiazza
- Resolved by Alessio Caiazza
@nolith Almost there, but it would help to have some tests for that code as it seems to me now that it is not working :)
assigned to @nolith
- Resolved by Alessio Caiazza
assigned to @ayufan
- Resolved by Alessio Caiazza
- Resolved by Alessio Caiazza
- Resolved by Alessio Caiazza
- Resolved by Alessio Caiazza
- Resolved by Alessio Caiazza
- Resolved by Kamil Trzciński
@nolith Awesome work! We only need to solve the certificate fetching, as we should not hardcode one.
assigned to @nolith
mentioned in issue #2815 (closed)
assigned to @ayufan
@ayufan I left a comment on !687 (comment 42529285) everything else should be covered.
mentioned in commit 0003707d
mentioned in issue #3497
mentioned in issue #2868 (closed)
mentioned in issue #1569 (closed)