-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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.
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
ui: Namespace Support #6639
ui: Namespace Support #6639
Changes from all commits
404fbc4
73f4d29
61a4d3e
c56b9ac
db852e8
4f00dc3
24e9ffe
0dbcc4e
8800e09
8167ada
3bd18d6
3e21667
6fde689
2c9445e
eb08c1f
822f426
b694b77
1a6b923
8e065c8
373dd78
4530d41
a15bda9
1a889cc
6459a8c
0f362f7
3aca683
4f124fe
f279418
29dcf06
8844a25
a89825f
d6a899f
3c4ffc4
73c9a83
d876982
f694ff3
edd83b6
9961ada
f929130
8181a90
23611c8
a670804
9ad76f7
f8adc3f
600e379
6dab00d
1c1107c
a6d0d23
b0eeb59
7933df7
1a9a1fc
c2df164
5362932
7fe5811
10215ab
885e98e
f649d06
619c0a7
fce23da
871db69
fd421ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,22 @@ | ||
import Adapter from './http'; | ||
import { inject as service } from '@ember/service'; | ||
import config from 'consul-ui/config/environment'; | ||
|
||
export const DATACENTER_QUERY_PARAM = 'dc'; | ||
export const NSPACE_QUERY_PARAM = 'ns'; | ||
export default Adapter.extend({ | ||
repo: service('settings'), | ||
client: service('client/http'), | ||
formatNspace: function(nspace) { | ||
if (config.CONSUL_NSPACES_ENABLED) { | ||
return nspace !== '' ? { [NSPACE_QUERY_PARAM]: nspace } : undefined; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does this need to be url encoded? wasn't sure if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't strictly correct 😁 . Basically its not When writing the template literal any variables used in the URL itself is encoded. Then any body used in anything other that a GET request is sent as JSON encoded. A GET request with a body is a little special and follows the old ember-data adapter functionality, in that any body with a GET request gets moved to the GET params (as you can't send a body with a GET request). Under the hood ember-data relied on jQuery to do this and when we did our data layer refactor we tried to replicate this functionality completely, you can see it here with a comment on urlencoding https://github.com/hashicorp/consul/pull/5637/files#diff-d12a13b0f2b1e3556993c370c1ee3ddfR182. I've just double checked this and it does get url encoded, but we are also protected somewhat by the fact that namespaces can only have DNS compatible names, and therefore probably don't need url encoding anyhow |
||
} | ||
}, | ||
formatDatacenter: function(dc) { | ||
return { | ||
[DATACENTER_QUERY_PARAM]: dc, | ||
}; | ||
}, | ||
// TODO: kinda protected for the moment | ||
// decide where this should go either read/write from http | ||
// should somehow use this or vice versa | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,10 @@ | ||
import Adapter, { DATACENTER_QUERY_PARAM as API_DATACENTER_KEY } from './application'; | ||
import { FOREIGN_KEY as DATACENTER_KEY } from 'consul-ui/models/dc'; | ||
import { SLUG_KEY } from 'consul-ui/models/intention'; | ||
// Intentions use SourceNS and DestinationNS properties for namespacing | ||
// so we don't need to add the `?ns=` anywhere here | ||
|
||
// TODO: Update to use this.formatDatacenter() | ||
export default Adapter.extend({ | ||
requestForQuery: function(request, { dc, index, id }) { | ||
return request` | ||
|
@@ -24,14 +28,30 @@ export default Adapter.extend({ | |
return request` | ||
POST /v1/connect/intentions?${{ [API_DATACENTER_KEY]: data[DATACENTER_KEY] }} | ||
|
||
${serialized} | ||
${{ | ||
SourceNS: serialized.SourceNS, | ||
DestinationNS: serialized.DestinationNS, | ||
SourceName: serialized.SourceName, | ||
DestinationName: serialized.DestinationName, | ||
SourceType: serialized.SourceType, | ||
Action: serialized.Action, | ||
Description: serialized.Description, | ||
}} | ||
`; | ||
}, | ||
requestForUpdateRecord: function(request, serialized, data) { | ||
return request` | ||
PUT /v1/connect/intentions/${data[SLUG_KEY]}?${{ [API_DATACENTER_KEY]: data[DATACENTER_KEY] }} | ||
|
||
${serialized} | ||
${{ | ||
SourceNS: serialized.SourceNS, | ||
DestinationNS: serialized.DestinationNS, | ||
SourceName: serialized.SourceName, | ||
DestinationName: serialized.DestinationName, | ||
SourceType: serialized.SourceType, | ||
Action: serialized.Action, | ||
Description: serialized.Description, | ||
}} | ||
`; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This means we can easily prevent certain props/values from being sent in a request, and means we can get rid of our custom There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you use serializer at all with the new adapter? Just curious as in out-of-the-box ember data, that's where this would be done. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We use our old style ApplicationSerializer, but we don't normalize anything there, the serializer deals with all the HTTP header bits still but not a lot else. Long term we hope to move everything to this style of normalization as its a easier to see whats happening, and looks exactly like the Consul API documentation, um more notes here: We still plan on a similar refactoring of our Serializers (similar to what we did for our Adapters) for response serializing, just its not high on the priority list right now, it might happen pre-1.8 or something. |
||
}, | ||
requestForDeleteRecord: function(request, serialized, data) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
import Adapter from './application'; | ||
import { SLUG_KEY } from 'consul-ui/models/nspace'; | ||
|
||
// namespaces aren't categorized by datacenter, therefore no dc | ||
export default Adapter.extend({ | ||
requestForQuery: function(request, { index }) { | ||
return request` | ||
GET /v1/namespaces | ||
|
||
${{ index }} | ||
`; | ||
}, | ||
requestForQueryRecord: function(request, { index, id }) { | ||
if (typeof id === 'undefined') { | ||
throw new Error('You must specify an name'); | ||
} | ||
return request` | ||
GET /v1/namespace/${id} | ||
|
||
${{ index }} | ||
`; | ||
}, | ||
requestForCreateRecord: function(request, serialized, data) { | ||
return request` | ||
PUT /v1/namespace/${data[SLUG_KEY]} | ||
|
||
${{ | ||
Name: serialized.Name, | ||
Description: serialized.Description, | ||
ACLs: { | ||
PolicyDefaults: serialized.ACLs.PolicyDefaults.map(item => ({ ID: item.ID })), | ||
RoleDefaults: serialized.ACLs.RoleDefaults.map(item => ({ ID: item.ID })), | ||
}, | ||
}} | ||
`; | ||
}, | ||
requestForUpdateRecord: function(request, serialized, data) { | ||
return request` | ||
PUT /v1/namespace/${data[SLUG_KEY]} | ||
|
||
${{ | ||
Description: serialized.Description, | ||
ACLs: { | ||
PolicyDefaults: serialized.ACLs.PolicyDefaults.map(item => ({ ID: item.ID })), | ||
RoleDefaults: serialized.ACLs.RoleDefaults.map(item => ({ ID: item.ID })), | ||
}, | ||
}} | ||
`; | ||
}, | ||
requestForDeleteRecord: function(request, serialized, data) { | ||
return request` | ||
DELETE /v1/namespace/${data[SLUG_KEY]} | ||
`; | ||
}, | ||
requestForAuthorize: function(request, { dc, ns, index }) { | ||
return request` | ||
POST /v1/internal/acl/authorize?${{ dc, ns, index }} | ||
|
||
${[ | ||
{ | ||
Resource: 'operator', | ||
Access: 'write', | ||
}, | ||
]} | ||
`; | ||
}, | ||
authorize: function(store, type, id, snapshot) { | ||
return this.request( | ||
function(adapter, request, serialized, unserialized) { | ||
return adapter.requestForAuthorize(request, serialized, unserialized); | ||
}, | ||
function(serializer, respond, serialized, unserialized) { | ||
// Completely skip the serializer here | ||
return respond(function(headers, body) { | ||
return body; | ||
}); | ||
}, | ||
snapshot, | ||
type.modelName | ||
); | ||
}, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,17 @@ | ||
import Adapter from './application'; | ||
// TODO: Update to use this.formatDatacenter() | ||
export default Adapter.extend({ | ||
requestForQuery: function(request, { dc, index, id }) { | ||
requestForQuery: function(request, { dc, ns, index, id }) { | ||
if (typeof id === 'undefined') { | ||
throw new Error('You must specify an id'); | ||
} | ||
return request` | ||
GET /v1/catalog/connect/${id}?${{ dc }} | ||
|
||
${{ index }} | ||
${{ | ||
...this.formatNspace(ns), | ||
index, | ||
}} | ||
`; | ||
}, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the enterprise binary write this value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will do at somepoint yeah, I think for now we can just use our CONSUL_BINARY_TYPE setting that we already have - I think this specific NSPACE one is going to be added later. Potentially a PR coming after this one to temporarily use the CONSUL_BINARY_TYPE setting (which the binary writes already)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has been PRed now, see #6921 (backend support) and #6933 (using these values properly)