Merge pull request #692 from rhcarvalho/must-compile

Merged by openshift-bot
openshift · Feb 24, 2017 · d76ef1e · d76ef1e
2 parents 52b113b + c0b819e
commit d76ef1e
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 2 deletions.
diff --git a/pkg/build/strategies/layered/layered.go b/pkg/build/strategies/layered/layered.go
@@ -42,12 +42,19 @@ type Layered struct {
 
 // New creates a Layered builder.
 func New(config *api.Config, fs util.FileSystem, scripts build.ScriptsHandler, overrides build.Overrides) (*Layered, error) {
+	excludePattern, err := regexp.Compile(config.ExcludeRegExp)
+	if err != nil {
+		return nil, err
+	}
+
 	d, err := docker.New(config.DockerConfig, config.PullAuthentication)
 	if err != nil {
 		return nil, err
 	}
+
 	tarHandler := tar.New(fs)
-	tarHandler.SetExclusionPattern(regexp.MustCompile(config.ExcludeRegExp))
+	tarHandler.SetExclusionPattern(excludePattern)
+
 	return &Layered{
 		docker:  d,
 		config:  config,

diff --git a/pkg/build/strategies/layered/layered_test.go b/pkg/build/strategies/layered/layered_test.go
@@ -6,10 +6,12 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"regexp/syntax"
 	"strings"
 	"testing"
 
 	"github.com/openshift/source-to-image/pkg/api"
+	"github.com/openshift/source-to-image/pkg/build"
 	"github.com/openshift/source-to-image/pkg/docker"
 	"github.com/openshift/source-to-image/pkg/test"
 )
@@ -185,3 +187,13 @@ func TestBuildErrorOnBuildBlocked(t *testing.T) {
 		t.Errorf("expected error from onbuild due to blocked ONBUILD, got: %v", err)
 	}
 }
+
+func TestNewWithInvalidExcludeRegExp(t *testing.T) {
+	_, err := New(&api.Config{
+		DockerConfig:  docker.GetDefaultDockerConfig(),
+		ExcludeRegExp: "[",
+	}, nil, nil, build.Overrides{})
+	if syntaxErr, ok := err.(*syntax.Error); ok && syntaxErr.Code != syntax.ErrMissingBracket {
+		t.Errorf("expected regexp compilation error, got %v", err)
+	}
+}
diff --git a/pkg/build/strategies/sti/sti.go b/pkg/build/strategies/sti/sti.go
@@ -86,6 +86,11 @@ type STI struct {
 // be used for the case that the base Docker image does not have 'tar' or 'bash'
 // installed.
 func New(config *api.Config, fs util.FileSystem, overrides build.Overrides) (*STI, error) {
+	excludePattern, err := regexp.Compile(config.ExcludeRegExp)
+	if err != nil {
+		return nil, err
+	}
+
 	docker, err := dockerpkg.New(config.DockerConfig, config.PullAuthentication)
 	if err != nil {
 		return nil, err
@@ -107,7 +112,7 @@ func New(config *api.Config, fs util.FileSystem, overrides build.Overrides) (*ST
 		fs,
 	)
 	tarHandler := tar.New(fs)
-	tarHandler.SetExclusionPattern(regexp.MustCompile(config.ExcludeRegExp))
+	tarHandler.SetExclusionPattern(excludePattern)
 
 	builder := &STI{
 		installer:              inst,

diff --git a/pkg/build/strategies/sti/sti_test.go b/pkg/build/strategies/sti/sti_test.go
@@ -6,6 +6,7 @@ import (
 	"io"
 	"path/filepath"
 	"reflect"
+	"regexp/syntax"
 	"strings"
 	"testing"
 
@@ -905,3 +906,13 @@ func TestCleanup(t *testing.T) {
 		}
 	}
 }
+
+func TestNewWithInvalidExcludeRegExp(t *testing.T) {
+	_, err := New(&api.Config{
+		DockerConfig:  docker.GetDefaultDockerConfig(),
+		ExcludeRegExp: "(",
+	}, nil, build.Overrides{})
+	if syntaxErr, ok := err.(*syntax.Error); ok && syntaxErr.Code != syntax.ErrMissingParen {
+		t.Errorf("expected regexp compilation error, got %v", err)
+	}
+}