Browse code

leverage new source-to-image API around git clone spec validation/correction

gabemontero authored on 2015/11/03 04:53:52
Showing 8 changed files
... ...
@@ -149,7 +149,6 @@ func (s *STIBuilder) Build() error {
149 149
 
150 150
 		BuilderImage: s.build.Spec.Strategy.SourceStrategy.From.Name,
151 151
 		Incremental:  s.build.Spec.Strategy.SourceStrategy.Incremental,
152
-		ForcePull:    s.build.Spec.Strategy.SourceStrategy.ForcePull,
153 152
 
154 153
 		Environment:       buildEnvVars(s.build),
155 154
 		DockerNetworkMode: getDockerNetworkMode(),
... ...
@@ -159,6 +158,16 @@ func (s *STIBuilder) Build() error {
159 159
 		ContextDir: s.build.Spec.Source.ContextDir,
160 160
 	}
161 161
 
162
+	if s.build.Spec.Strategy.SourceStrategy.ForcePull {
163
+		glog.V(4).Infof("With force pull true, setting policies to %s", s2iapi.PullAlways)
164
+		config.PreviousImagePullPolicy = s2iapi.PullAlways
165
+		config.BuilderPullPolicy = s2iapi.PullAlways
166
+	} else {
167
+		glog.V(4).Infof("With force pull false, setting policies to %s", s2iapi.PullIfNotPresent)
168
+		config.PreviousImagePullPolicy = s2iapi.PullIfNotPresent
169
+		config.BuilderPullPolicy = s2iapi.PullIfNotPresent
170
+	}
171
+
162 172
 	allowedUIDs := os.Getenv("ALLOWED_UIDS")
163 173
 	glog.V(2).Infof("The value of ALLOWED_UIDS is [%s]", allowedUIDs)
164 174
 	if len(allowedUIDs) > 0 {
... ...
@@ -66,8 +66,8 @@ func TestAddArguments(t *testing.T) {
66 66
 			unknown:    []string{},
67 67
 		},
68 68
 		"source": {
69
-			args:    []string{".", testDir, "git://server/repo.git"},
70
-			repos:   util.StringList([]string{".", testDir, "git://server/repo.git"}),
69
+			args:    []string{".", testDir, "git://github.com/openshift/origin.git"},
70
+			repos:   util.StringList([]string{".", testDir, "git://github.com/openshift/origin.git"}),
71 71
 			unknown: []string{},
72 72
 		},
73 73
 		"env": {
... ...
@@ -76,9 +76,9 @@ func TestAddArguments(t *testing.T) {
76 76
 			unknown: []string{},
77 77
 		},
78 78
 		"mix 1": {
79
-			args:       []string{"git://server/repo.git", "mysql+ruby~git@test.server/repo.git", "env1=test", "ruby-helloworld-sample"},
80
-			repos:      util.StringList([]string{"git://server/repo.git"}),
81
-			components: util.StringList([]string{"mysql+ruby~git@test.server/repo.git", "ruby-helloworld-sample"}),
79
+			args:       []string{"git://github.com/openshift/origin.git", "mysql+ruby~git@github.com/openshift/origin.git", "env1=test", "ruby-helloworld-sample"},
80
+			repos:      util.StringList([]string{"git://github.com/openshift/origin.git"}),
81
+			components: util.StringList([]string{"mysql+ruby~git@github.com/openshift/origin.git", "ruby-helloworld-sample"}),
82 82
 			env:        util.StringList([]string{"env1=test"}),
83 83
 			unknown:    []string{},
84 84
 		},
... ...
@@ -256,6 +256,9 @@ func TestBuildTemplates(t *testing.T) {
256 256
 }
257 257
 
258 258
 func TestEnsureHasSource(t *testing.T) {
259
+	gitLocalDir := createLocalGitDirectory(t)
260
+	defer os.RemoveAll(gitLocalDir)
261
+
259 262
 	tests := []struct {
260 263
 		name              string
261 264
 		cfg               AppConfig
... ...
@@ -271,7 +274,7 @@ func TestEnsureHasSource(t *testing.T) {
271 271
 					ExpectToBuild: true,
272 272
 				}),
273 273
 			},
274
-			repositories: MockSourceRepositories(t),
274
+			repositories: MockSourceRepositories(t, gitLocalDir),
275 275
 			expectedErr:  "there are multiple code locations provided - use one of the following suggestions",
276 276
 		},
277 277
 		{
... ...
@@ -284,7 +287,7 @@ func TestEnsureHasSource(t *testing.T) {
284 284
 					ExpectToBuild: true,
285 285
 				}),
286 286
 			},
287
-			repositories: MockSourceRepositories(t),
287
+			repositories: MockSourceRepositories(t, gitLocalDir),
288 288
 			expectedErr:  "Use '[image]~[repo]' to declare which code goes with which image",
289 289
 		},
290 290
 		{
... ...
@@ -319,7 +322,7 @@ func TestEnsureHasSource(t *testing.T) {
319 319
 					ExpectToBuild: false,
320 320
 				}),
321 321
 			},
322
-			repositories: MockSourceRepositories(t)[:1],
322
+			repositories: MockSourceRepositories(t, gitLocalDir)[:1],
323 323
 			expectedErr:  "",
324 324
 		},
325 325
 		{
... ...
@@ -329,7 +332,7 @@ func TestEnsureHasSource(t *testing.T) {
329 329
 					ExpectToBuild: false,
330 330
 				}),
331 331
 			},
332
-			repositories: MockSourceRepositories(t),
332
+			repositories: MockSourceRepositories(t, gitLocalDir),
333 333
 			expectedErr:  "",
334 334
 		},
335 335
 	}
... ...
@@ -421,10 +424,13 @@ func TestResolve(t *testing.T) {
421 421
 
422 422
 func TestDetectSource(t *testing.T) {
423 423
 	skipExternalGit(t)
424
+	gitLocalDir := createLocalGitDirectory(t)
425
+	defer os.RemoveAll(gitLocalDir)
426
+
424 427
 	dockerSearcher := app.DockerRegistrySearcher{
425 428
 		Client: dockerregistry.NewClient(),
426 429
 	}
427
-	mocks := MockSourceRepositories(t)
430
+	mocks := MockSourceRepositories(t, gitLocalDir)
428 431
 	tests := []struct {
429 432
 		name         string
430 433
 		cfg          *AppConfig
... ...
@@ -441,7 +447,7 @@ func TestDetectSource(t *testing.T) {
441 441
 				},
442 442
 				dockerSearcher: dockerSearcher,
443 443
 			},
444
-			repositories: []*app.SourceRepository{mocks[1]},
444
+			repositories: []*app.SourceRepository{mocks[0]},
445 445
 			expectedLang: "ruby",
446 446
 			expectedErr:  "",
447 447
 		},
... ...
@@ -1503,14 +1509,22 @@ func fakeSimpleDockerSearcher() app.Searcher {
1503 1503
 	}
1504 1504
 }
1505 1505
 
1506
+func createLocalGitDirectory(t *testing.T) string {
1507
+	dir, err := ioutil.TempDir(os.TempDir(), "s2i-test")
1508
+	if err != nil {
1509
+		t.Error(err)
1510
+	}
1511
+	os.Mkdir(filepath.Join(dir, ".git"), 0600)
1512
+	return dir
1513
+}
1514
+
1506 1515
 // MockSourceRepositories is a set of mocked source repositories used for
1507 1516
 // testing
1508
-func MockSourceRepositories(t *testing.T) []*app.SourceRepository {
1517
+func MockSourceRepositories(t *testing.T, file string) []*app.SourceRepository {
1509 1518
 	var b []*app.SourceRepository
1510 1519
 	for _, location := range []string{
1511
-		"some/location.git",
1512 1520
 		"https://github.com/openshift/ruby-hello-world.git",
1513
-		"another/location.git",
1521
+		file,
1514 1522
 	} {
1515 1523
 		s, err := app.NewSourceRepository(location)
1516 1524
 		if err != nil {
... ...
@@ -5,7 +5,6 @@ import (
5 5
 	"io/ioutil"
6 6
 	"net/url"
7 7
 	"path/filepath"
8
-	"regexp"
9 8
 	"strings"
10 9
 
11 10
 	"github.com/docker/docker/builder/parser"
... ...
@@ -13,11 +12,7 @@ import (
13 13
 	"github.com/openshift/origin/pkg/generate/dockerfile"
14 14
 	"github.com/openshift/origin/pkg/generate/git"
15 15
 	"github.com/openshift/origin/pkg/generate/source"
16
-)
17
-
18
-var (
19
-	argumentGit         = regexp.MustCompile("^(http://|https://|git@|git://).*(?:#([a-zA-Z0-9]*))?$")
20
-	argumentGitProtocol = regexp.MustCompile("^(git@|git://)")
16
+	s2igit "github.com/openshift/source-to-image/pkg/scm/git"
21 17
 )
22 18
 
23 19
 type Dockerfile interface {
... ...
@@ -67,7 +62,14 @@ func IsPossibleSourceRepository(s string) bool {
67 67
 
68 68
 // IsRemoteRepository checks whether the provided string is a remote repository or not
69 69
 func IsRemoteRepository(s string) bool {
70
-	return argumentGit.MatchString(s) || argumentGitProtocol.MatchString(s)
70
+	if !s2igit.New().ValidCloneSpecRemoteOnly(s) {
71
+		return false
72
+	}
73
+	gitRepo := git.NewRepository()
74
+	if err := gitRepo.ListRemote(s); err != nil {
75
+		return false
76
+	}
77
+	return true
71 78
 }
72 79
 
73 80
 // SourceRepository represents a code repository that may be the target of a build.
... ...
@@ -66,3 +66,7 @@ func (f *FakeGit) AddRemote(source, remote, url string) error {
66 66
 func (f *FakeGit) ShowFormat(source, ref, format string) (string, error) {
67 67
 	return "", nil
68 68
 }
69
+
70
+func (f *FakeGit) ListRemote(url string) error {
71
+	return nil
72
+}
... ...
@@ -2,10 +2,10 @@ package git
2 2
 
3 3
 import (
4 4
 	"bufio"
5
+	s2igit "github.com/openshift/source-to-image/pkg/scm/git"
5 6
 	"io"
6 7
 	"net/url"
7 8
 	"path"
8
-	"path/filepath"
9 9
 	"strings"
10 10
 )
11 11
 
... ...
@@ -16,29 +16,25 @@ import (
16 16
 // - http, https
17 17
 // - file
18 18
 // - git
19
+// - ssh
19 20
 func ParseRepository(s string) (*url.URL, error) {
20 21
 	uri, err := url.Parse(s)
21 22
 	if err != nil {
22 23
 		return nil, err
23 24
 	}
24 25
 
25
-	if uri.Scheme == "" && !strings.HasPrefix(uri.Path, "git@") {
26
-		path := s
27
-		ref := ""
28
-		segments := strings.SplitN(path, "#", 2)
29
-		if len(segments) == 2 {
30
-			path, ref = segments[0], segments[1]
31
-		}
32
-		path, err := filepath.Abs(path)
33
-		if err != nil {
34
-			return nil, err
35
-		}
36
-		uri = &url.URL{
37
-			Scheme:   "file",
38
-			Path:     path,
39
-			Fragment: ref,
40
-		}
26
+	// There are some shortcomings with url.Parse when it comes to GIT, namely wrt
27
+	// the GIT local/file and ssh protocols - it does not handle implied schema (i.e. no <proto>:// prefix)well;
28
+	// We handle those caveats here
29
+	err = s2igit.New().MungeNoProtocolURL(s, uri)
30
+	if err != nil {
31
+		return nil, err
41 32
 	}
33
+	//TODO temporary work around for s2i's MungeNoProtocolURL (which is not used in s2i at the moment)
34
+	// there is some overloaded usage between the uri Path and the Path needed for DownloaderForSource
35
+	// this temporary work around  can sit here even after the s2i change (including duplicating tests in this dir's git_test.go  to source-to-image/pkg/scm/git/git_test.go)
36
+	// gets Godeps into origin, and then we can remove this
37
+	uri.Path = strings.TrimPrefix(uri.Path, "file://")
42 38
 
43 39
 	return uri, nil
44 40
 }
45 41
new file mode 100644
... ...
@@ -0,0 +1,132 @@
0
+package git
1
+
2
+import (
3
+	"io/ioutil"
4
+	"net/url"
5
+	"os"
6
+	"path/filepath"
7
+	"testing"
8
+)
9
+
10
+func createLocalGitDirectory(t *testing.T) string {
11
+	dir, err := ioutil.TempDir(os.TempDir(), "s2i-test")
12
+	if err != nil {
13
+		t.Error(err)
14
+	}
15
+	os.Mkdir(filepath.Join(dir, ".git"), 0600)
16
+	return dir
17
+}
18
+
19
+func TestParseRepository(t *testing.T) {
20
+	gitLocalDir := createLocalGitDirectory(t)
21
+	defer os.RemoveAll(gitLocalDir)
22
+
23
+	tests := map[string]url.URL{
24
+		"git@github.com:user/repo.git": {
25
+			Scheme: "ssh",
26
+			Host:   "github.com",
27
+			User:   url.User("git"),
28
+			Path:   "user/repo.git",
29
+		},
30
+		"git://github.com/user/repo.git": {
31
+			Scheme: "git",
32
+			Host:   "github.com",
33
+			Path:   "/user/repo.git",
34
+		},
35
+		"git://github.com/user/repo": {
36
+			Scheme: "git",
37
+			Host:   "github.com",
38
+			Path:   "/user/repo",
39
+		},
40
+		"http://github.com/user/repo.git": {
41
+			Scheme: "http",
42
+			Host:   "github.com",
43
+			Path:   "/user/repo.git",
44
+		},
45
+		"http://github.com/user/repo": {
46
+			Scheme: "http",
47
+			Host:   "github.com",
48
+			Path:   "/user/repo",
49
+		},
50
+		"https://github.com/user/repo.git": {
51
+			Scheme: "https",
52
+			Host:   "github.com",
53
+			Path:   "/user/repo.git",
54
+		},
55
+		"https://github.com/user/repo": {
56
+			Scheme: "https",
57
+			Host:   "github.com",
58
+			Path:   "/user/repo",
59
+		},
60
+		"file://" + gitLocalDir: {
61
+			Scheme: "file",
62
+			Path:   gitLocalDir,
63
+		},
64
+		gitLocalDir: {
65
+			Scheme: "file",
66
+			Path:   gitLocalDir,
67
+		},
68
+		"git@192.168.122.1:repositories/authooks": {
69
+			Scheme: "ssh",
70
+			Host:   "192.168.122.1",
71
+			User:   url.User("git"),
72
+			Path:   "repositories/authooks",
73
+		},
74
+		"mbalazs@build.ulx.hu:/var/git/eap-ulx.git": {
75
+			Scheme: "ssh",
76
+			Host:   "build.ulx.hu",
77
+			User:   url.User("mbalazs"),
78
+			Path:   "/var/git/eap-ulx.git",
79
+		},
80
+		"ssh://git@[2001:db8::1]/repository.git": {
81
+			Scheme: "ssh",
82
+			Host:   "[2001:db8::1]",
83
+			User:   url.User("git"),
84
+			Path:   "/repository.git",
85
+		},
86
+		"ssh://git@mydomain.com:8080/foo/bar": {
87
+			Scheme: "ssh",
88
+			Host:   "mydomain.com:8080",
89
+			User:   url.User("git"),
90
+			Path:   "/foo/bar",
91
+		},
92
+		"git@[2001:db8::1]:repository.git": {
93
+			Scheme: "ssh",
94
+			Host:   "[2001:db8::1]",
95
+			User:   url.User("git"),
96
+			Path:   "repository.git",
97
+		},
98
+		"git@[2001:db8::1]:/repository.git": {
99
+			Scheme: "ssh",
100
+			Host:   "[2001:db8::1]",
101
+			User:   url.User("git"),
102
+			Path:   "/repository.git",
103
+		},
104
+	}
105
+
106
+	for scenario, test := range tests {
107
+		out, err := ParseRepository(scenario)
108
+		if err != nil {
109
+			t.Errorf("ParseRepository returned err: %v", err)
110
+		}
111
+
112
+		// reflect.DeepEqual was not dealing with url.URL correctly, have to check each field individually
113
+		// First, the easy string compares
114
+		equal := out.Scheme == test.Scheme && out.Opaque == test.Opaque && out.Host == test.Host && out.Path == test.Path && out.RawQuery == test.RawQuery && out.Fragment == test.Fragment
115
+		if equal {
116
+			// now deal with User, a Userinfo struct ptr
117
+			if out.User == nil && test.User != nil {
118
+				equal = false
119
+			} else if out.User != nil && test.User == nil {
120
+				equal = false
121
+			} else if out.User != nil && test.User != nil {
122
+				equal = out.User.String() == test.User.String()
123
+			}
124
+		}
125
+		if !equal {
126
+			t.Errorf("For URL string %s, field by field check for scheme %v opaque %v host %v path %v rawq %v frag %v out user nil %v test user nil %v out scheme  %s out opaque %s out host %s out path %s  out raw query %s out frag %s", scenario,
127
+				out.Scheme == test.Scheme, out.Opaque == test.Opaque, out.Host == test.Host, out.Path == test.Path, out.RawQuery == test.RawQuery,
128
+				out.Fragment == test.Fragment, out.User == nil, test.User == nil, out.Scheme, out.Opaque, out.Host, out.Path, out.RawQuery, out.Fragment)
129
+		}
130
+	}
131
+}
... ...
@@ -28,6 +28,7 @@ type Repository interface {
28 28
 	AddRemote(dir string, name, url string) error
29 29
 	AddLocalConfig(dir, name, value string) error
30 30
 	ShowFormat(dir, commit, format string) (string, error)
31
+	ListRemote(url string) error
31 32
 }
32 33
 
33 34
 // execGitFunc is a function that executes a Git command
... ...
@@ -147,6 +148,12 @@ func (r *repository) Clone(location string, url string) error {
147 147
 	return err
148 148
 }
149 149
 
150
+// LsRemote lists references in a remote repository
151
+func (r *repository) ListRemote(url string) error {
152
+	_, _, err := r.git(nil, "", "ls-remote", url)
153
+	return err
154
+}
155
+
150 156
 // CloneMirror clones a remote git repository to a local directory as a mirror
151 157
 func (r *repository) CloneMirror(location string, url string) error {
152 158
 	_, _, err := r.git(nil, "", "clone", "--mirror", url, location)
... ...
@@ -88,7 +88,7 @@ oc delete all -l app=helloworld
88 88
 [ "$(oc new-app ruby-helloworld-sample -l app=helloworld -o name 2>&1 | grep 'Service/frontend')" ]
89 89
 oc delete all -l app=helloworld
90 90
 # create from template with code explicitly set is not supported
91
-[ ! "$(oc new-app ruby-helloworld-sample~git@github.com/mfojtik/sinatra-app-example)" ]
91
+[ ! "$(oc new-app ruby-helloworld-sample~git@github.com:mfojtik/sinatra-app-example)" ]
92 92
 oc delete template ruby-helloworld-sample
93 93
 # override component names
94 94
 [ "$(oc new-app mysql --name=db | grep db)" ]