fix(whitelist): Handle empty whitelist from user gracefully#1045
fix(whitelist): Handle empty whitelist from user gracefully#1045kmala merged 1 commit intodeis:masterfrom
Conversation
|
@helgi, @bacongobbler and @mboersma are potential reviewers of this pull request based on my analysis of |
| addresses = ",".join(address for address in whitelist) | ||
| service['metadata']['annotations']['router.deis.io/whitelist'] = addresses | ||
| elif 'router.deis.io/whitelist' in service['metadata']['annotations']: | ||
| service['metadata']['annotations'].pop('router.deis.io/whitelist', None) |
There was a problem hiding this comment.
don't we want to pop, then update it? Not sure how this is relevant for #1044
There was a problem hiding this comment.
pop should fix deis/router#246 as i don't want to have the value empty.
| whitelist = ArrayField(models.CharField(max_length=50), default=[]) | ||
| # the default values is None to differentiate from user sending an empty whitelist | ||
| # and user just updating other fields meaning the values needs to be copied from prev release | ||
| whitelist = ArrayField(models.CharField(max_length=50), default=None) |
There was a problem hiding this comment.
Won't this break the client since it expects an array? deis whitelist:list would break on an app with no whitelist set.
https://github.com/deis/workflow-cli/blob/master/cmd/whitelist.go#L24-L26
https://github.com/deis/controller-sdk-go/blob/master/api/appsettings.go#L25
There was a problem hiding this comment.
this is just the default value, the value saved in database will be empty array....i am doing it in update_whitelist function which is called before saving to database.
There was a problem hiding this comment.
Ah I see what this does now. Same problem as with routable and maintenance mode... Need three values to differentiate between values unchanged (None), set and delete.
Current coverage is 86.92% (diff: 80.00%)@@ master #1045 diff @@
==========================================
Files 42 42
Lines 3559 3563 +4
Methods 0 0
Messages 0 0
Branches 604 606 +2
==========================================
+ Hits 3090 3097 +7
+ Misses 309 307 -2
+ Partials 160 159 -1
|
6 participants
fixes #1044