Skip to content

Commit 7816a26

Browse files
committed
Improve route form
* Make "Split traffic across multiple services" a checkbox like "Secure route" * Add consistent section headings for "Alernate Services" and "Security" * Fix bug where `insecureEdgeTerminationPolicy` was initialized to an invalid value, causing an error creating edge terminated routes * Fix a bug where the range slider was not properly initialized for existing values
1 parent ddb780e commit 7816a26

File tree

4 files changed

+139
-79
lines changed

4 files changed

+139
-79
lines changed

Diff for: app/scripts/directives/oscRouting.js

+62-23
Original file line numberDiff line numberDiff line change
@@ -41,22 +41,31 @@ angular.module("openshiftConsole")
4141
hostReadOnly: "="
4242
},
4343
templateUrl: 'views/directives/osc-routing.html',
44-
controller: function($scope) {
45-
$scope.disableCertificateInputs = function() {
46-
var termination = _.get($scope, 'route.tls.termination');
44+
link: function(scope, element, attrs, formCtl) {
45+
scope.form = formCtl;
46+
scope.controls = {};
47+
scope.options = {
48+
secureRoute: false,
49+
alternateServices: false
50+
};
51+
52+
scope.disableWildcards = Constants.DISABLE_WILDCARD_ROUTES;
53+
scope.disableCertificateInputs = function() {
54+
var termination = _.get(scope, 'route.tls.termination');
4755
return !termination || termination === 'passthrough';
4856
};
49-
$scope.insecureTrafficOptions = [
57+
58+
scope.insecureTrafficOptions = [
5059
{value: '', label: 'None'},
5160
{value: 'Allow', label: 'Allow'},
5261
{value: 'Redirect', label: 'Redirect'}
5362
];
54-
},
55-
link: function(scope, element, attrs, formCtl) {
56-
scope.form = formCtl;
57-
scope.controls = {};
5863

59-
scope.disableWildcards = Constants.DISABLE_WILDCARD_ROUTES;
64+
if (!_.has(scope, 'route.tls.insecureEdgeTerminationPolicy')) {
65+
// Initialize the value to the empty string so the option 'None' is
66+
// shown in the select.
67+
_.set(scope, 'route.tls.insecureEdgeTerminationPolicy', '');
68+
}
6069

6170
// Use different patterns for validating hostnames if wildcard subdomains are supported.
6271
if (scope.disableWildcards) {
@@ -119,6 +128,8 @@ angular.module("openshiftConsole")
119128
return _.includes(iteratee, value, index + 1);
120129
}).value();
121130
formCtl.$setValidity("duplicateServices", !scope.duplicateServices.length);
131+
132+
scope.options.alternateServices = !_.isEmpty(alternateServices);
122133
}, true);
123134

124135
var showCertificateWarning = function() {
@@ -140,52 +151,61 @@ angular.module("openshiftConsole")
140151
// Show a warning if previously-set certificates won't be used because
141152
// the TLS termination is now incompatible.
142153
scope.$watch('route.tls.termination', function() {
143-
scope.secureRoute = !!_.get(scope, 'route.tls.termination');
154+
scope.options.secureRoute = !!_.get(scope, 'route.tls.termination');
144155
scope.showCertificatesNotUsedWarning = showCertificateWarning();
145156
});
146157

147158
var previousTermination;
148-
scope.$watch('secureRoute', function(newValue, oldValue) {
159+
scope.$watch('options.secureRoute', function(newValue, oldValue) {
149160
if (newValue === oldValue) {
150161
return;
151162
}
152163

153-
// Set the default behavior of insecure connections to 'None'
154-
if (newValue && !_.get(scope, 'route.tls.insecureEdgeTerminationPolicy')) {
155-
_.set(scope, 'route.tls.insecureEdgeTerminationPolicy', scope.insecureTrafficOptions[0]);
156-
}
157-
158164
var termination = _.get(scope, 'route.tls.termination');
159165
if (!scope.securetRoute && termination) {
160166
// Remember previous value if user switches back to secure.
161167
previousTermination = termination;
162168
delete scope.route.tls.termination;
163169
}
164170

165-
if (scope.secureRoute && !termination) {
171+
if (scope.options.secureRoute && !termination) {
166172
// Restore previous termination value or default to edge if no previous value.
167173
_.set(scope, 'route.tls.termination', previousTermination || 'edge');
168174
}
169175
});
170176

177+
scope.$watch('options.alternateServices', function(alternateServices, previousValue) {
178+
if (alternateServices === previousValue) {
179+
return;
180+
}
181+
182+
if (!alternateServices) {
183+
scope.route.alternateServices = [];
184+
}
185+
186+
if (alternateServices && _.isEmpty(scope.route.alternateServices)) {
187+
scope.addAlternateService();
188+
}
189+
});
190+
171191
scope.addAlternateService = function() {
172192
scope.route.alternateServices = scope.route.alternateServices || [];
173193
var firstUnselected = _.find(scope.services, function(service) {
174194
return service !== scope.route.to.service && !_.some(scope.route.alternateServices, { service: service });
175195
});
176196

197+
if (!_.has(scope, 'route.to.weight')) {
198+
_.set(scope, 'route.to.weight', 1);
199+
}
200+
177201
// Add a new value.
178202
scope.route.alternateServices.push({
179203
service: firstUnselected,
180204
weight: 1
181205
});
182-
183-
if (!_.has(scope, 'route.to.weight')) {
184-
_.set(scope, 'route.to.weight', 1);
185-
}
186206
};
187207

188-
scope.weightAsPercentage = function(weight) {
208+
scope.weightAsPercentage = function(weight, format) {
189209
weight = weight || 0;
190210

191211
var total = _.get(scope, 'route.to.weight', 0);
@@ -198,10 +218,29 @@ angular.module("openshiftConsole")
198218
}
199219

200220
var percentage = (weight / total) * 100;
201-
return d3.round(percentage, 1) + '%';
221+
return format ? (d3.round(percentage, 1) + '%') : percentage;
202222
};
203223

224+
var initializingSlider = false;
225+
scope.$watch('route.alternateServices.length', function(alternateServicesCount) {
226+
if (alternateServicesCount === 0 && _.has(scope, 'route.to.weight')) {
227+
// Reset the primary service weight. This rebalances the percentages when adding a new alternate service.
228+
delete scope.route.to.weight;
229+
}
230+
231+
if (alternateServicesCount === 1) {
232+
initializingSlider = true;
233+
scope.controls.rangeSlider = scope.weightAsPercentage(scope.route.to.weight);
234+
}
235+
});
236+
204237
scope.$watch('controls.rangeSlider', function(weight, previous) {
238+
// Don't update the routes if we're setting the initial slider value.
239+
if (initializingSlider) {
240+
initializingSlider = false;
241+
return;
242+
}
243+
205244
if (weight === previous) {
206245
return;
207246
}

Diff for: app/views/directives/osc-routing.html

+27-18
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,6 @@
113113
</osc-routing-service>
114114
</div>
115115

116-
<div ng-if="alternateServiceOptions.length && !route.alternateServices.length" class="form-group">
117-
<a href="" ng-click="addAlternateService()">Split traffic across multiple services</a>
118-
</div>
119-
120116
<!-- Target Port -->
121117
<div ng-if="route.portOptions.length" class="form-group">
122118
<label for="routeTargetPort">Target Port</label>
@@ -133,9 +129,23 @@
133129
</div>
134130
</div>
135131

132+
136133
<!-- Alternate Services for A/B Traffic -->
137-
<div ng-if="route.alternateServices.length">
134+
<div ng-if="alternateServiceOptions.length">
138135
<h3>Alternate Services</h3>
136+
<div class="form-group">
137+
<div class="checkbox">
138+
<label>
139+
<input type="checkbox" ng-model="options.alternateServices" aria-describedby="secure-route-help">
140+
Split traffic across multiple services
141+
</label>
142+
<div class="help-block">
143+
Routes can direct traffic to multiple services for A/B testing. Each service has a weight
144+
controlling how much traffic it gets.
145+
</div>
146+
</div>
147+
</div>
148+
139149
<div ng-repeat="alternate in route.alternateServices" class="form-group">
140150
<osc-routing-service model="alternate"
141151
services="alternateServiceOptions"
@@ -162,12 +172,12 @@ <h3>Service Weights</h3>
162172
<div class="weight-slider-values">
163173
<div>
164174
<span class="service-name">{{route.to.service.metadata.name}}</span>
165-
<span class="weight-percentage">{{weightAsPercentage(route.to.weight)}}</span>
175+
<span class="weight-percentage">{{weightAsPercentage(route.to.weight, true)}}</span>
166176
</div>
167177
<div>
168-
<span class="weight-percentage hidden-xs">{{weightAsPercentage(route.alternateServices[0].weight)}}</span>
178+
<span class="weight-percentage hidden-xs">{{weightAsPercentage(route.alternateServices[0].weight, true)}}</span>
169179
<span class="service-name">{{route.alternateServices[0].service.metadata.name}}</span>
170-
<span class="weight-percentage visible-xs-inline">{{weightAsPercentage(route.alternateServices[0].weight)}}</span>
180+
<span class="weight-percentage visible-xs-inline">{{weightAsPercentage(route.alternateServices[0].weight, true)}}</span>
171181
</div>
172182
</div>
173183
<label class="sr-only" for="weight-slider">Service {{route.to.service.metadata.name}} Weight</label>
@@ -195,17 +205,18 @@ <h3>Service Weights</h3>
195205
</div>
196206
</div>
197207

208+
<h3>Security</h3>
198209
<div class="checkbox">
199210
<label>
200-
<input type="checkbox" ng-model="secureRoute" aria-describedby="secure-route-help">
211+
<input type="checkbox" ng-model="options.secureRoute" aria-describedby="secure-route-help">
201212
Secure route
202213
</label>
203214
<div class="help-block" id="secure-route-help">
204215
Routes can be secured using several TLS termination types for serving certificates.
205216
</div>
206217
</div>
207218

208-
<div ng-show="secureRoute">
219+
<div ng-show="options.secureRoute">
209220
<!-- TLS Termination -->
210221
<div class="form-group">
211222
<label for="tlsTermination">TLS Termination</label>
@@ -238,11 +249,9 @@ <h3>Service Weights</h3>
238249

239250
<!-- Key and Certificates -->
240251
<h3>Certificates</h3>
241-
<div>
242-
<span class="help-block">
243-
TLS certificates for edge and re-encrypt termination. If not
244-
specified, the router's default certificate is used.
245-
</span>
252+
<div class="help-block">
253+
TLS certificates for edge and re-encrypt termination. If not specified, the router's default
254+
certificate is used.
246255
</div>
247256
<div ng-if="showCertificatesNotUsedWarning" class="has-warning">
248257
<span class="help-block">
@@ -255,7 +264,7 @@ <h3>Certificates</h3>
255264
</span>
256265
</span>
257266
</div>
258-
<fieldset>
267+
<fieldset class="mar-top-md">
259268
<div>
260269
<div class="form-group" id="certificate-file">
261270
<label>Certificate</label>
@@ -300,8 +309,8 @@ <h3>Certificates</h3>
300309
not already showing the generic certificate warning above. -->
301310
<div ng-if="route.tls.destinationCACertificate && route.tls.termination !== 'reencrypt' && !showCertificatesNotUsedWarning" class="has-warning">
302311
<span class="help-block">
303-
The destination CA certificate will not be used. Destination CA
304-
certificates are only used for re-encrypt termination.
312+
The destination CA certificate will be removed from the route.
313+
Destination CA certificates are only used for re-encrypt termination.
305314
</span>
306315
</div>
307316
</div>

Diff for: dist/scripts/scripts.js

+29-24
Original file line numberDiff line numberDiff line change
@@ -9905,11 +9905,14 @@ routingDisabled:"=",
99059905
hostReadOnly:"="
99069906
},
99079907
templateUrl:"views/directives/osc-routing.html",
9908-
controller:[ "$scope", function(a) {
9909-
a.disableCertificateInputs = function() {
9910-
var b = _.get(a, "route.tls.termination");
9911-
return !b || "passthrough" === b;
9912-
}, a.insecureTrafficOptions = [ {
9908+
link:function(b, c, d, e) {
9909+
b.form = e, b.controls = {}, b.options = {
9910+
secureRoute:!1,
9911+
alternateServices:!1
9912+
}, b.disableWildcards = a.DISABLE_WILDCARD_ROUTES, b.disableCertificateInputs = function() {
9913+
var a = _.get(b, "route.tls.termination");
9914+
return !a || "passthrough" === a;
9915+
}, b.insecureTrafficOptions = [ {
99139916
value:"",
99149917
label:"None"
99159918
}, {
@@ -9918,10 +9921,7 @@ label:"Allow"
99189921
}, {
99199922
value:"Redirect",
99209923
label:"Redirect"
9921-
} ];
9922-
} ],
9923-
link:function(b, c, d, e) {
9924-
b.form = e, b.controls = {}, b.disableWildcards = a.DISABLE_WILDCARD_ROUTES, b.disableWildcards ? b.hostnamePattern = /^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$/ :b.hostnamePattern = /^(\*(\.[a-z0-9]([-a-z0-9]*[a-z0-9]))+|[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*)$/;
9924+
} ], _.has(b, "route.tls.insecureEdgeTerminationPolicy") || _.set(b, "route.tls.insecureEdgeTerminationPolicy", ""), b.disableWildcards ? b.hostnamePattern = /^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$/ :b.hostnamePattern = /^(\*(\.[a-z0-9]([-a-z0-9]*[a-z0-9]))+|[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*)$/;
99259925
var f = function(a) {
99269926
a && (b.unnamedServicePort = 1 === a.spec.ports.length && !a.spec.ports[0].name, a.spec.ports.length && !b.unnamedServicePort ? b.route.portOptions = _.map(a.spec.ports, function(a) {
99279927
return {
@@ -9937,42 +9937,47 @@ return a === b;
99379937
}), b.$watch("route.alternateServices", function(a) {
99389938
b.duplicateServices = _(a).map("service").filter(function(a, b, c) {
99399939
return _.includes(c, a, b + 1);
9940-
}).value(), e.$setValidity("duplicateServices", !b.duplicateServices.length);
9940+
}).value(), e.$setValidity("duplicateServices", !b.duplicateServices.length), b.options.alternateServices = !_.isEmpty(a);
99419941
}, !0);
99429942
var g = function() {
99439943
return !!b.route.tls && ((!b.route.tls.termination || "passthrough" === b.route.tls.termination) && (b.route.tls.certificate || b.route.tls.key || b.route.tls.caCertificate || b.route.tls.destinationCACertificate));
99449944
};
99459945
b.$watch("route.tls.termination", function() {
9946-
b.secureRoute = !!_.get(b, "route.tls.termination"), b.showCertificatesNotUsedWarning = g();
9946+
b.options.secureRoute = !!_.get(b, "route.tls.termination"), b.showCertificatesNotUsedWarning = g();
99479947
});
99489948
var h;
9949-
b.$watch("secureRoute", function(a, c) {
9949+
b.$watch("options.secureRoute", function(a, c) {
99509950
if (a !== c) {
9951-
a && !_.get(b, "route.tls.insecureEdgeTerminationPolicy") && _.set(b, "route.tls.insecureEdgeTerminationPolicy", b.insecureTrafficOptions[0]);
99529951
var d = _.get(b, "route.tls.termination");
9953-
!b.securetRoute && d && (h = d, delete b.route.tls.termination), b.secureRoute && !d && _.set(b, "route.tls.termination", h || "edge");
9952+
!b.securetRoute && d && (h = d, delete b.route.tls.termination), b.options.secureRoute && !d && _.set(b, "route.tls.termination", h || "edge");
99549953
}
9954+
}), b.$watch("options.alternateServices", function(a, c) {
9955+
a !== c && (a || (b.route.alternateServices = []), a && _.isEmpty(b.route.alternateServices) && b.addAlternateService());
99559956
}), b.addAlternateService = function() {
99569957
b.route.alternateServices = b.route.alternateServices || [];
99579958
var a = _.find(b.services, function(a) {
99589959
return a !== b.route.to.service && !_.some(b.route.alternateServices, {
99599960
service:a
99609961
});
99619962
});
9962-
b.route.alternateServices.push({
9963+
_.has(b, "route.to.weight") || _.set(b, "route.to.weight", 1), b.route.alternateServices.push({
99639964
service:a,
99649965
weight:1
9965-
}), _.has(b, "route.to.weight") || _.set(b, "route.to.weight", 1);
9966-
}, b.weightAsPercentage = function(a) {
9966+
});
9967+
}, b.weightAsPercentage = function(a, c) {
99679968
a = a || 0;
9968-
var c = _.get(b, "route.to.weight", 0);
9969+
var d = _.get(b, "route.to.weight", 0);
99699970
if (_.each(b.route.alternateServices, function(a) {
9970-
c += _.get(a, "weight", 0);
9971-
}), !c) return "";
9972-
var d = a / c * 100;
9973-
return d3.round(d, 1) + "%";
9974-
}, b.$watch("controls.rangeSlider", function(a, c) {
9975-
a !== c && (a = parseInt(a, 10), _.set(b, "route.to.weight", a), _.set(b, "route.alternateServices[0].weight", 100 - a));
9971+
d += _.get(a, "weight", 0);
9972+
}), !d) return "";
9973+
var e = a / d * 100;
9974+
return c ? d3.round(e, 1) + "%" :e;
9975+
};
9976+
var i = !1;
9977+
b.$watch("route.alternateServices.length", function(a) {
9978+
0 === a && _.has(b, "route.to.weight") && delete b.route.to.weight, 1 === a && (i = !0, b.controls.rangeSlider = b.weightAsPercentage(b.route.to.weight));
9979+
}), b.$watch("controls.rangeSlider", function(a, c) {
9980+
return i ? void (i = !1) :void (a !== c && (a = parseInt(a, 10), _.set(b, "route.to.weight", a), _.set(b, "route.alternateServices[0].weight", 100 - a)));
99769981
});
99779982
}
99789983
};

0 commit comments

Comments
 (0)