Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1365986
Slava Semushin authored on 2016/10/25 02:38:24... | ... |
@@ -168,6 +168,7 @@ func (o *NewBuildOptions) Complete(baseName, commandName string, f *clientcmd.Fa |
168 | 168 |
o.Action.Bulk.Retry = retryBuildConfig |
169 | 169 |
|
170 | 170 |
o.Config.DryRun = o.Action.DryRun |
171 |
+ o.Config.AllowNonNumericExposedPorts = true |
|
171 | 172 |
|
172 | 173 |
o.BaseName = baseName |
173 | 174 |
o.CommandPath = c.CommandPath() |
... | ... |
@@ -6,6 +6,7 @@ import ( |
6 | 6 |
"fmt" |
7 | 7 |
"io" |
8 | 8 |
"reflect" |
9 |
+ "strconv" |
|
9 | 10 |
"strings" |
10 | 11 |
"time" |
11 | 12 |
|
... | ... |
@@ -20,6 +21,7 @@ import ( |
20 | 20 |
"k8s.io/kubernetes/pkg/runtime" |
21 | 21 |
kutilerrors "k8s.io/kubernetes/pkg/util/errors" |
22 | 22 |
|
23 |
+ dockerfileparser "github.com/docker/docker/builder/dockerfile/parser" |
|
23 | 24 |
authapi "github.com/openshift/origin/pkg/authorization/api" |
24 | 25 |
buildapi "github.com/openshift/origin/pkg/build/api" |
25 | 26 |
buildutil "github.com/openshift/origin/pkg/build/util" |
... | ... |
@@ -31,6 +33,7 @@ import ( |
31 | 31 |
"github.com/openshift/origin/pkg/generate/source" |
32 | 32 |
imageapi "github.com/openshift/origin/pkg/image/api" |
33 | 33 |
outil "github.com/openshift/origin/pkg/util" |
34 |
+ dockerfileutil "github.com/openshift/origin/pkg/util/docker/dockerfile" |
|
34 | 35 |
) |
35 | 36 |
|
36 | 37 |
const ( |
... | ... |
@@ -90,8 +93,9 @@ type AppConfig struct { |
90 | 90 |
|
91 | 91 |
SkipGeneration bool |
92 | 92 |
|
93 |
- AllowSecretUse bool |
|
94 |
- SecretAccessor app.SecretAccessor |
|
93 |
+ AllowSecretUse bool |
|
94 |
+ AllowNonNumericExposedPorts bool |
|
95 |
+ SecretAccessor app.SecretAccessor |
|
95 | 96 |
|
96 | 97 |
AsSearch bool |
97 | 98 |
AsList bool |
... | ... |
@@ -615,6 +619,10 @@ func (c *AppConfig) Run() (*AppResult, error) { |
615 | 615 |
} |
616 | 616 |
} |
617 | 617 |
|
618 |
+ if err := optionallyValidateExposedPorts(c, repositories); err != nil { |
|
619 |
+ return nil, err |
|
620 |
+ } |
|
621 |
+ |
|
618 | 622 |
if len(c.To) > 0 { |
619 | 623 |
if err := validateOutputImageReference(c.To); err != nil { |
620 | 624 |
return nil, err |
... | ... |
@@ -887,3 +895,33 @@ func (c *AppConfig) GetBuildEnvironment(environment app.Environment) app.Environ |
887 | 887 |
} |
888 | 888 |
return app.Environment{} |
889 | 889 |
} |
890 |
+ |
|
891 |
+func optionallyValidateExposedPorts(config *AppConfig, repositories app.SourceRepositories) error { |
|
892 |
+ if config.AllowNonNumericExposedPorts { |
|
893 |
+ return nil |
|
894 |
+ } |
|
895 |
+ |
|
896 |
+ if config.Strategy != "docker" { |
|
897 |
+ return nil |
|
898 |
+ } |
|
899 |
+ |
|
900 |
+ for _, repo := range repositories { |
|
901 |
+ if repoInfo := repo.Info(); repoInfo != nil && repoInfo.Dockerfile != nil { |
|
902 |
+ node := repoInfo.Dockerfile.AST() |
|
903 |
+ if err := exposedPortsAreNumeric(node); err != nil { |
|
904 |
+ return fmt.Errorf("the Dockerfile has an invalid EXPOSE instruction: %v", err) |
|
905 |
+ } |
|
906 |
+ } |
|
907 |
+ } |
|
908 |
+ |
|
909 |
+ return nil |
|
910 |
+} |
|
911 |
+ |
|
912 |
+func exposedPortsAreNumeric(node *dockerfileparser.Node) error { |
|
913 |
+ for _, port := range dockerfileutil.LastExposedPorts(node) { |
|
914 |
+ if _, err := strconv.ParseInt(port, 10, 32); err != nil { |
|
915 |
+ return fmt.Errorf("could not parse %q: must be numeric", port) |
|
916 |
+ } |
|
917 |
+ } |
|
918 |
+ return nil |
|
919 |
+} |
... | ... |
@@ -506,3 +506,57 @@ func TestBuildOutputCycleWithFollowingTag(t *testing.T) { |
506 | 506 |
t.Errorf("Expected error from followRefToDockerImage: got \"%v\" versus expected %q", err, expected) |
507 | 507 |
} |
508 | 508 |
} |
509 |
+ |
|
510 |
+func TestAllowedNonNumericExposedPorts(t *testing.T) { |
|
511 |
+ tests := []struct { |
|
512 |
+ strategy string |
|
513 |
+ allowNonNumericPorts bool |
|
514 |
+ }{ |
|
515 |
+ { |
|
516 |
+ strategy: "", |
|
517 |
+ allowNonNumericPorts: true, |
|
518 |
+ }, |
|
519 |
+ { |
|
520 |
+ strategy: "source", |
|
521 |
+ allowNonNumericPorts: false, |
|
522 |
+ }, |
|
523 |
+ } |
|
524 |
+ |
|
525 |
+ for _, test := range tests { |
|
526 |
+ config := &AppConfig{} |
|
527 |
+ config.Strategy = test.strategy |
|
528 |
+ config.AllowNonNumericExposedPorts = test.allowNonNumericPorts |
|
529 |
+ |
|
530 |
+ repo, err := app.NewSourceRepositoryForDockerfile("FROM centos\nARG PORT=80\nEXPOSE $PORT") |
|
531 |
+ if err != nil { |
|
532 |
+ t.Errorf("Unexpected error during setup: %v", err) |
|
533 |
+ continue |
|
534 |
+ } |
|
535 |
+ repos := app.SourceRepositories{repo} |
|
536 |
+ |
|
537 |
+ err = optionallyValidateExposedPorts(config, repos) |
|
538 |
+ if err != nil { |
|
539 |
+ t.Errorf("Unexpected error: %v", err) |
|
540 |
+ } |
|
541 |
+ } |
|
542 |
+} |
|
543 |
+ |
|
544 |
+func TestDisallowedNonNumericExposedPorts(t *testing.T) { |
|
545 |
+ config := &AppConfig{} |
|
546 |
+ config.Strategy = "docker" |
|
547 |
+ config.AllowNonNumericExposedPorts = false |
|
548 |
+ |
|
549 |
+ repo, err := app.NewSourceRepositoryForDockerfile("FROM centos\nARG PORT=80\nEXPOSE 8080 $PORT") |
|
550 |
+ if err != nil { |
|
551 |
+ t.Fatalf("Unexpected error during setup: %v", err) |
|
552 |
+ } |
|
553 |
+ repos := app.SourceRepositories{repo} |
|
554 |
+ |
|
555 |
+ err = optionallyValidateExposedPorts(config, repos) |
|
556 |
+ if err == nil { |
|
557 |
+ t.Error("Expected error wasn't returned") |
|
558 |
+ |
|
559 |
+ } else if !strings.Contains(err.Error(), "invalid EXPOSE") || !strings.Contains(err.Error(), "must be numeric") { |
|
560 |
+ t.Errorf("Unexpected error: %v", err) |
|
561 |
+ } |
|
562 |
+} |