Skip to content

Warn when Git URL is not an absolute URL #977

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 2, 2016
Merged

Warn when Git URL is not an absolute URL #977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion app/scripts/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,11 @@ angular
screenLgMin: 1200, // screen-lg
screenXlgMin: 1600 // screen-xlg
})
.constant('SOURCE_URL_PATTERN', /^((ftp|http|https|git|ssh):\/\/(\w+:{0,1}[^\s@]*@)|git@)?([^\s@]+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?$/ )
// A (very) basic regex to determine if a URL is an absolute URL, enough to
// warn the user the Git URL probably won't work. This should only be used
// as a sanity test and shouldn't block submitting the form. Rely on the API
// server for any additional validation.
.constant('SOURCE_URL_PATTERN', /^[a-z][a-z0-9+.-@]*:(\/\/)?[0-9a-z_-]+/i)
// http://stackoverflow.com/questions/9038625/detect-if-device-is-ios
.constant('IS_IOS', /iPad|iPhone|iPod/.test(navigator.userAgent) && !window.MSStream)
.constant('amTimeAgoConfig', {
Expand Down
1 change: 0 additions & 1 deletion app/scripts/controllers/edit/deploymentConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ angular.module('openshiftConsole')
Navigate,
ProjectsService,
SecretsService,
SOURCE_URL_PATTERN,
keyValueEditorUtils) {
$scope.projectName = $routeParams.project;
$scope.deploymentConfig = null;
Expand Down
6 changes: 3 additions & 3 deletions app/views/create/fromimage.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
'col-lg-12': !advancedOptions}">
<div class="form-group">
<label for="sourceUrl" class="required">Git Repository URL</label>
<div ng-class="{'has-warning': form.sourceUrl.$dirty && !sourceURLPattern.test(buildConfig.sourceUrl), 'has-error': (form.sourceUrl.$error.required && form.sourceUrl.$dirty)}">
<div ng-class="{'has-warning': form.sourceUrl.$touched && !sourceURLPattern.test(buildConfig.sourceUrl), 'has-error': (form.sourceUrl.$error.required && form.sourceUrl.$dirty)}">
<!-- Unfortunately, we can't set type="url" because some valid values don't pass browser validation. -->
<input class="form-control"
id="sourceUrl"
Expand All @@ -98,8 +98,8 @@
<div class="has-error" ng-show="form.sourceUrl.$error.required && form.sourceUrl.$dirty">
<span class="help-block">A Git repository URL is required.</span>
</div>
<div>
<span class="text-warning" ng-if="form.sourceUrl.$dirty && !sourceURLPattern.test(buildConfig.sourceUrl)">Git repository should be a URL.</span>
<div class="has-warning" ng-if="buildConfig.sourceUrl && form.sourceUrl.$touched && !sourceURLPattern.test(buildConfig.sourceUrl)">
<span class="help-block">This might not be a valid Git URL. Check that it is the correct URL to a remote Git repository.</span>
</div>
</div>
</div>
Expand Down
6 changes: 3 additions & 3 deletions app/views/edit/build-config.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@ <h3>Source Configuration</h3>
aria-describedby="source-url-help"
required>
</div>
<div>
<span class="text-warning" ng-if="form.sourceUrl.$dirty && !sourceURLPattern.test(updatedBuildConfig.spec.source.git.uri)">Git repository should be a URL.</span>
</div>
<div class="help-block" id="source-url-help">
Git URL of the source code to build.
<span ng-if="!view.advancedOptions">If your Git repository is private, view the <a href="" ng-click="view.advancedOptions = true">advanced options</a> to set up authentication.</span>
</div>
<div class="has-warning" ng-if="updatedBuildConfig.spec.source.git.uri && form.sourceUrl.$touched && !sourceURLPattern.test(buildConfig.sourceUrl)">
<span class="help-block">This might not be a valid Git URL. Check that it is the correct URL to a remote Git repository.</span>
</div>
</div>

</div>
Expand Down
36 changes: 18 additions & 18 deletions dist/scripts/scripts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ screenSmMin:768,
screenMdMin:992,
screenLgMin:1200,
screenXlgMin:1600
}).constant("SOURCE_URL_PATTERN", /^((ftp|http|https|git|ssh):\/\/(\w+:{0,1}[^\s@]*@)|git@)?([^\s@]+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?$/).constant("IS_IOS", /iPad|iPhone|iPod/.test(navigator.userAgent) && !window.MSStream).constant("amTimeAgoConfig", {
}).constant("SOURCE_URL_PATTERN", /^[a-z][a-z0-9+.-@]*:(\/\/)?[0-9a-z_-]+/i).constant("IS_IOS", /iPad|iPhone|iPod/.test(navigator.userAgent) && !window.MSStream).constant("amTimeAgoConfig", {
titleFormat:"LLL"
}).config([ "$httpProvider", "AuthServiceProvider", "RedirectLoginServiceProvider", "AUTH_CFG", "API_CFG", "kubernetesContainerSocketProvider", function(a, b, c, d, e, f) {
a.interceptors.push("AuthInterceptor"), b.LoginService("RedirectLoginService"), b.LogoutService("DeleteTokenLogoutService"), b.UserStore("LocalStorageUserStore"), c.OAuthClientID(d.oauth_client_id), c.OAuthAuthorizeURI(d.oauth_authorize_uri), c.OAuthRedirectURI(URI(d.oauth_redirect_base).segment("oauth").toString()), f.WebSocketFactory = "ContainerWebSocket";
Expand Down Expand Up @@ -6943,7 +6943,7 @@ details:a("getErrorDetails")(b)
e.unwatchAll(i);
});
}));
} ]), angular.module("openshiftConsole").controller("EditDeploymentConfigController", [ "$scope", "$filter", "$location", "$routeParams", "$uibModal", "AlertMessageService", "AuthorizationService", "BreadcrumbsService", "DataService", "Navigate", "ProjectsService", "SecretsService", "SOURCE_URL_PATTERN", "keyValueEditorUtils", function(a, b, c, d, e, f, g, h, i, j, k, l, m, n) {
} ]), angular.module("openshiftConsole").controller("EditDeploymentConfigController", [ "$scope", "$filter", "$location", "$routeParams", "$uibModal", "AlertMessageService", "AuthorizationService", "BreadcrumbsService", "DataService", "Navigate", "ProjectsService", "SecretsService", "keyValueEditorUtils", function(a, b, c, d, e, f, g, h, i, j, k, l, m) {
a.projectName = d.project, a.deploymentConfig = null, a.alerts = {}, a.view = {
advancedStrategyOptions:!1,
advancedImageOptions:!1
Expand All @@ -6956,7 +6956,7 @@ includeProject:!0
}), a.deploymentConfigStrategyTypes = [ "Recreate", "Rolling", "Custom" ], f.getAlerts().forEach(function(b) {
a.alerts[b.name] = b.data;
}), f.clearAlerts();
var o = [], p = function(a) {
var n = [], o = function(a) {
switch (a) {
case "Recreate":
return "recreateParams";
Expand Down Expand Up @@ -7018,7 +7018,7 @@ a.updatedDeploymentConfig = angular.copy(a.deploymentConfig), a.containerNames =
pullSecrets:angular.copy(a.deploymentConfig.spec.template.spec.imagePullSecrets) || [ {
name:""
} ]
}, a.volumeNames = _.map(a.deploymentConfig.spec.template.spec.volumes, "name"), a.strategyData = angular.copy(a.deploymentConfig.spec.strategy), a.originalStrategy = a.strategyData.type, a.strategyParamsPropertyName = p(a.strategyData.type), a.triggers.hasConfigTrigger = _.some(a.updatedDeploymentConfig.spec.triggers, {
}, a.volumeNames = _.map(a.deploymentConfig.spec.template.spec.volumes, "name"), a.strategyData = angular.copy(a.deploymentConfig.spec.strategy), a.originalStrategy = a.strategyData.type, a.strategyParamsPropertyName = o(a.strategyData.type), a.triggers.hasConfigTrigger = _.some(a.updatedDeploymentConfig.spec.triggers, {
type:"ConfigChange"
}), "Custom" !== a.strategyData.type || _.has(a.strategyData, "customParams.environment") || (a.strategyData.customParams.environment = []), i.list("secrets", e, function(b) {
var c = l.groupSecretsByType(b), d = _.mapValues(c, function(a) {
Expand All @@ -7027,7 +7027,7 @@ return _.map(a, "metadata.name");
a.secretsByType = _.each(d, function(a) {
a.unshift("");
});
}), o.push(i.watchObject("deploymentconfigs", d.deploymentconfig, e, function(b, c) {
}), n.push(i.watchObject("deploymentconfigs", d.deploymentconfig, e, function(b, c) {
"MODIFIED" === c && (a.alerts["updated/deleted"] = {
type:"warning",
message:"This deployment configuration has changed since you started editing it. You'll need to copy any changes you've made and edit again."
Expand All @@ -7044,9 +7044,9 @@ details:b("getErrorDetails")(c)
};
}) :void j.toErrorPage("You do not have authority to update deployment config " + d.deploymentconfig + ".", "access_denied");
}));
var q = function() {
var p = function() {
return "Custom" !== a.strategyData.type && "Custom" !== a.originalStrategy && a.strategyData.type !== a.originalStrategy;
}, r = function(b) {
}, q = function(b) {
if (!_.has(a.strategyData, b)) {
var c = e.open({
animation:!0,
Expand All @@ -7066,21 +7066,21 @@ cancelButtonText:"No"
}
});
c.result.then(function() {
a.strategyData[b] = angular.copy(a.strategyData[p(a.originalStrategy)]);
a.strategyData[b] = angular.copy(a.strategyData[o(a.originalStrategy)]);
}, function() {
a.strategyData[b] = {};
});
}
};
a.strategyChanged = function() {
var b = p(a.strategyData.type);
q() ? r(b) :_.has(a.strategyData, b) || ("Custom" !== a.strategyData.type ? a.strategyData[b] = {} :a.strategyData[b] = {
var b = o(a.strategyData.type);
p() ? q(b) :_.has(a.strategyData, b) || ("Custom" !== a.strategyData.type ? a.strategyData[b] = {} :a.strategyData[b] = {
image:"",
command:[],
environment:[]
}), a.strategyParamsPropertyName = b;
};
var s = function(a, b, c, d) {
var r = function(a, b, c, d) {
var e = {
kind:"ImageStreamTag",
namespace:b.namespace,
Expand All @@ -7094,12 +7094,12 @@ containerNames:[ a ],
from:e
}
}, c;
}, t = function() {
}, s = function() {
var b = _.reject(a.updatedDeploymentConfig.spec.triggers, function(a) {
return "ImageChange" === a.type || "ConfigChange" === a.type;
});
return _.each(a.containerConfigByName, function(c, d) {
if (c.hasDeploymentTrigger) b.push(s(d, c.triggerData.istag, c.triggerData.data, c.triggerData.automatic)); else {
if (c.hasDeploymentTrigger) b.push(r(d, c.triggerData.istag, c.triggerData.data, c.triggerData.automatic)); else {
var e = _.find(a.updatedDeploymentConfig.spec.template.spec.containers, {
name:d
});
Expand All @@ -7114,10 +7114,10 @@ a.disableInputs = !0, _.each(a.containerConfigByName, function(b, c) {
var d = _.find(a.updatedDeploymentConfig.spec.template.spec.containers, {
name:c
});
d.env = n.compactEntries(b.env);
}), q() && delete a.strategyData[p(a.originalStrategy)], "Custom" !== a.strategyData.type ? _.each([ "pre", "mid", "post" ], function(b) {
_.has(a.strategyData, [ a.strategyParamsPropertyName, b, "execNewPod", "env" ]) && (a.strategyData[a.strategyParamsPropertyName][b].execNewPod.env = n.compactEntries(a.strategyData[a.strategyParamsPropertyName][b].execNewPod.env));
}) :_.has(a, "strategyData.customParams.environment") && (a.strategyData.customParams.environment = n.compactEntries(a.strategyData.customParams.environment)), a.updatedDeploymentConfig.spec.template.spec.imagePullSecrets = _.filter(a.secrets.pullSecrets, "name"), a.updatedDeploymentConfig.spec.strategy = a.strategyData, a.updatedDeploymentConfig.spec.triggers = t(), i.update("deploymentconfigs", a.updatedDeploymentConfig.metadata.name, a.updatedDeploymentConfig, a.context).then(function() {
d.env = m.compactEntries(b.env);
}), p() && delete a.strategyData[o(a.originalStrategy)], "Custom" !== a.strategyData.type ? _.each([ "pre", "mid", "post" ], function(b) {
_.has(a.strategyData, [ a.strategyParamsPropertyName, b, "execNewPod", "env" ]) && (a.strategyData[a.strategyParamsPropertyName][b].execNewPod.env = m.compactEntries(a.strategyData[a.strategyParamsPropertyName][b].execNewPod.env));
}) :_.has(a, "strategyData.customParams.environment") && (a.strategyData.customParams.environment = m.compactEntries(a.strategyData.customParams.environment)), a.updatedDeploymentConfig.spec.template.spec.imagePullSecrets = _.filter(a.secrets.pullSecrets, "name"), a.updatedDeploymentConfig.spec.strategy = a.strategyData, a.updatedDeploymentConfig.spec.triggers = s(), i.update("deploymentconfigs", a.updatedDeploymentConfig.metadata.name, a.updatedDeploymentConfig, a.context).then(function() {
f.addAlert({
name:a.updatedDeploymentConfig.metadata.name,
data:{
Expand All @@ -7135,7 +7135,7 @@ details:b("getErrorDetails")(c)
};
});
}, a.$on("$destroy", function() {
i.unwatchAll(o);
i.unwatchAll(n);
});
} ]), angular.module("openshiftConsole").controller("EditAutoscalerController", [ "$scope", "$filter", "$routeParams", "$window", "APIService", "AuthorizationService", "BreadcrumbsService", "DataService", "HPAService", "MetricsService", "Navigate", "ProjectsService", "keyValueEditorUtils", function(a, b, c, d, e, f, g, h, i, j, k, l, m) {
if (!c.kind || !c.name) return void k.toErrorPage("Kind or name parameter missing.");
Expand Down
12 changes: 6 additions & 6 deletions dist/scripts/templates.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4656,7 +4656,7 @@ angular.module('openshiftConsoleTemplates', []).run(['$templateCache', function(
" 'col-lg-12': !advancedOptions}\">\n" +
"<div class=\"form-group\">\n" +
"<label for=\"sourceUrl\" class=\"required\">Git Repository URL</label>\n" +
"<div ng-class=\"{'has-warning': form.sourceUrl.$dirty && !sourceURLPattern.test(buildConfig.sourceUrl), 'has-error': (form.sourceUrl.$error.required && form.sourceUrl.$dirty)}\">\n" +
"<div ng-class=\"{'has-warning': form.sourceUrl.$touched && !sourceURLPattern.test(buildConfig.sourceUrl), 'has-error': (form.sourceUrl.$error.required && form.sourceUrl.$dirty)}\">\n" +
"\n" +
"<input class=\"form-control\" id=\"sourceUrl\" name=\"sourceUrl\" type=\"text\" required aria-describedby=\"from_source_help\" ng-model=\"buildConfig.sourceUrl\" autocorrect=\"off\" autocapitalize=\"off\" spellcheck=\"false\">\n" +
"</div>\n" +
Expand All @@ -4667,8 +4667,8 @@ angular.module('openshiftConsoleTemplates', []).run(['$templateCache', function(
"<div class=\"has-error\" ng-show=\"form.sourceUrl.$error.required && form.sourceUrl.$dirty\">\n" +
"<span class=\"help-block\">A Git repository URL is required.</span>\n" +
"</div>\n" +
"<div>\n" +
"<span class=\"text-warning\" ng-if=\"form.sourceUrl.$dirty && !sourceURLPattern.test(buildConfig.sourceUrl)\">Git repository should be a URL.</span>\n" +
"<div class=\"has-warning\" ng-if=\"buildConfig.sourceUrl && form.sourceUrl.$touched && !sourceURLPattern.test(buildConfig.sourceUrl)\">\n" +
"<span class=\"help-block\">This might not be a valid Git URL. Check that it is the correct URL to a remote Git repository.</span>\n" +
"</div>\n" +
"</div>\n" +
"</div>\n" +
Expand Down Expand Up @@ -8038,13 +8038,13 @@ angular.module('openshiftConsoleTemplates', []).run(['$templateCache', function(
"\n" +
"<input class=\"form-control\" id=\"sourceUrl\" name=\"sourceUrl\" ng-model=\"updatedBuildConfig.spec.source.git.uri\" type=\"text\" autocorrect=\"off\" autocapitalize=\"off\" spellcheck=\"false\" aria-describedby=\"source-url-help\" required>\n" +
"</div>\n" +
"<div>\n" +
"<span class=\"text-warning\" ng-if=\"form.sourceUrl.$dirty && !sourceURLPattern.test(updatedBuildConfig.spec.source.git.uri)\">Git repository should be a URL.</span>\n" +
"</div>\n" +
"<div class=\"help-block\" id=\"source-url-help\">\n" +
"Git URL of the source code to build.\n" +
"<span ng-if=\"!view.advancedOptions\">If your Git repository is private, view the <a href=\"\" ng-click=\"view.advancedOptions = true\">advanced options</a> to set up authentication.</span>\n" +
"</div>\n" +
"<div class=\"has-warning\" ng-if=\"updatedBuildConfig.spec.source.git.uri && form.sourceUrl.$touched && !sourceURLPattern.test(buildConfig.sourceUrl)\">\n" +
"<span class=\"help-block\">This might not be a valid Git URL. Check that it is the correct URL to a remote Git repository.</span>\n" +
"</div>\n" +
"</div>\n" +
"</div>\n" +
"<div class=\"col-lg-4\" ng-if=\"view.advancedOptions\">\n" +
Expand Down
17 changes: 3 additions & 14 deletions test/spec/controllers/create/createFromImageSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,22 +69,16 @@ describe("CreateFromImageController", function(){
expect(match).not.toBeNull();
});

it("valid http URL, without http part", function(){
it("invalid http URL, without http part", function(){
var match = 'example.com/dir1/dir2'.match($scope.sourceURLPattern);
expect(match).not.toBeNull();
expect(match).toBeNull();
});


it("valid http URL with user and password", function(){
var match = 'http://user:[email protected]/dir1/dir2'.match($scope.sourceURLPattern);
expect(match).not.toBeNull();
});

it("invalid http URL with user and password containing space", function(){
var match = 'http://user:pa [email protected]/dir1/dir2'.match($scope.sourceURLPattern);
expect(match).toBeNull();
});

it("valid http URL with user and password with special characters", function(){
var match = 'https://user:[email protected]/gerrit/p/myrepo.git'.match($scope.sourceURLPattern);
expect(match).not.toBeNull();
Expand Down Expand Up @@ -115,11 +109,6 @@ describe("CreateFromImageController", function(){
expect(match).not.toBeNull();
});

it("invalid git+ssh URL (double @@)", function(){
var match = 'git@@example.com:dir1/dir2'.match($scope.sourceURLPattern);
expect(match).toBeNull();
});

it("valid ssh URL", function(){
var match = 'ssh://[email protected]/dir1/dir2'.match($scope.sourceURLPattern);
expect(match).not.toBeNull();
Expand All @@ -129,5 +118,5 @@ describe("CreateFromImageController", function(){
var match = 'ssh://[email protected]:8080/dir1/dir2'.match($scope.sourceURLPattern);
expect(match).not.toBeNull();
});

});