Skip to content

Add dynamic and static IP/MAC lookups #157

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
anhou merged 4 commits into from
Mar 1, 2016
Merged

Add dynamic and static IP/MAC lookups #157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
092f665
Add dynamic and static IP/MAC lookups
jlongever Feb 28, 2016
a4c6c8f
Add missing semicolon
jlongever Feb 28, 2016
ccfbed9
add query parameter descriptions
jlongever Mar 1, 2016
db3bf8a
Add some coverage
jlongever Mar 1, 2016
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
26 changes: 23 additions & 3 deletions lib/api/1.1/southbound/profiles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,49 @@ di.annotate(profilesRouterFactory, new di.Provide('Http.Api.Internal.Profiles'))
di.annotate(profilesRouterFactory,
new di.Inject(
'Http.Services.Api.Profiles',
'common-api-presenter'
'common-api-presenter',
'Logger'
)
);

function profilesRouterFactory (
profileApiService,
presenter
presenter,
Logger
) {
var router = express.Router();
var logger = Logger.initialize(profilesRouterFactory);

/**
* @api {get} /api/1.1/profiles GET /
* @apiVersion 1.1.0
* @apiParam {query} macs List of valid MAC addresses to lookup
* @apiParam {query} mac When macs parameter is not passed,
* passed with IP adds MAC address to lookup
* @apiParam {query} ip When macs parameters is not passed,
* passed with MAC adds IP address to lookup
* @apiDescription used internally by the system -- will NOT get a list of all profiles,
* look at api/current/profiles/library
* @apiName profiles-get
* @apiGroup profiles
*/

router.get('/profiles', function (req, res) {
if (req.query.mac && req.query.ip) {
Copy link
Member

@anhou anhou Mar 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you help move the description like 'Creates lookup entry based on DHCP bootfile option request query parameters' to the annotations here? the message in PR wouldn't be included in codes, and annotations here will help a lot for knowing why having this codes, and also for code readablility,

Copy link
Contributor Author

@jlongever jlongever Mar 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anhou Are you looking for comments? There are other routes missing query parameter annotations; maybe there should be a story to track updating those as well.

Copy link
Member

@anhou anhou Mar 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, not for the routes, but for if() {... profileApiService.setLookup...} 's

Copy link
Contributor Author

@jlongever jlongever Mar 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, there are a lot of route code that reference query parameters that offer no description.

profileApiService.setLookup(req.query)
.catch(function(err) {
logger.error('Error Setting Lookup', {
mac: req.params.mac,
ip: req.params.ip,
error: err
});
res.status(500).json(err);
});
req.query.macs = req.query.mac;
}

if (req.query.macs) {
var macAddresses = profileApiService.getMacs(req.query.macs);

return profileApiService.getNode(macAddresses)
.then(function (node) {
if (node.newRecord) {
Expand Down
14 changes: 14 additions & 0 deletions lib/services/profiles-api-service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ di.annotate(profileApiServiceFactory,
'Protocol.Events',
'Services.Waterline',
'Services.Configuration',
'Services.Lookup',
'Logger',
'Errors',
'_'
Expand All @@ -26,6 +27,7 @@ function profileApiServiceFactory(
eventsProtocol,
waterline,
configuration,
lookupService,
Logger,
Errors,
_
Expand Down Expand Up @@ -61,6 +63,18 @@ function profileApiServiceFactory(
return _.flattenDeep([macs]);
};

ProfileApiService.prototype.setLookup = function(query) {
return waterline.nodes.findByIdentifier(this.getMacs(query.mac))
.then(function (node) {
if (_.isUndefined(node)) {
return lookupService.setIpAddress(
query.ip,
query.mac
);
}
});
};

ProfileApiService.prototype.getNode = function(macAddresses) {
var self = this;
return waterline.nodes.findByIdentifier(macAddresses)
Expand Down
15 changes: 15 additions & 0 deletions spec/lib/api/1.1/profiles-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ describe('Http.Api.Profiles', function () {
sinon.stub(profileApiService, 'getNode').resolves({});
sinon.stub(profileApiService, 'createNodeAndRunDiscovery').resolves({});
sinon.stub(profileApiService, 'runDiscovery').resolves({});
sinon.stub(profileApiService, 'setLookup').resolves();
});

afterEach('teardown mocks', function () {
Expand Down Expand Up @@ -117,6 +118,20 @@ describe('Http.Api.Profiles', function () {
});

describe("GET /profiles", function() {
it("should receive both mac and ip query", function() {
return helper.request().get('/api/1.1/profiles?mac=00:01:02:03:04:05&&ip=1.1.1.1')
.expect(200)
.expect(function() {
expect(profileApiService.setLookup).to.have.been.calledOnce;
});
});

it("should send 500 set mac and ip fails", function() {
profileApiService.setLookup.rejects();
return helper.request().get('/api/1.1/profiles?mac=00:01:02:03:04:05&&ip=1.1.1.1')
.expect(500);
});

it("should send down redirect.ipxe if 'macs' are not in req.query", function() {
profileApiService.getNode.restore();
return helper.request().get('/api/1.1/profiles')
Expand Down
26 changes: 13 additions & 13 deletions spec/lib/api/login/login-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe('Http.Api.Login', function () {

var endpoint = {
"address": "0.0.0.0",
"port": 8443,
"port": 9443,
"httpsEnabled": true,
"httpsCert": "data/dev-cert.pem",
"httpsKey": "data/dev-key.pem",
Expand Down Expand Up @@ -85,7 +85,7 @@ describe('Http.Api.Login', function () {
});

it('should return a token with correct credential in request body', function() {
return helper.request('https://localhost:8443')
return helper.request('https://localhost:9443')
.post('/login')
.send({username: "admin", password: "admin123"})
.expect(SUCCESS_STATUS)
Expand All @@ -95,7 +95,7 @@ describe('Http.Api.Login', function () {
});

it('should fail with wrong username in request body', function() {
return helper.request('https://localhost:8443')
return helper.request('https://localhost:9443')
.post('/login')
.send({username: "balabalabala", password: "admin123"})
.expect(UNAUTHORIZED_STATUS)
Expand All @@ -106,7 +106,7 @@ describe('Http.Api.Login', function () {
});

it('should fail with wrong password in request body', function() {
return helper.request('https://localhost:8443')
return helper.request('https://localhost:9443')
.post('/login')
.send({username: "admin", password: "balabalabala"})
.expect(UNAUTHORIZED_STATUS)
Expand All @@ -117,7 +117,7 @@ describe('Http.Api.Login', function () {
});

it('should fail with empty username in request body', function() {
return helper.request('https://localhost:8443')
return helper.request('https://localhost:9443')
.post('/login')
.send({username: "", password: "admin123"})
.expect(BAD_REQUEST_STATUS)
Expand All @@ -128,7 +128,7 @@ describe('Http.Api.Login', function () {
});

it('should fail with empty password in request body', function() {
return helper.request('https://localhost:8443')
return helper.request('https://localhost:9443')
.post('/login')
.send({username: "admin", password: ""})
.expect(BAD_REQUEST_STATUS)
Expand All @@ -139,7 +139,7 @@ describe('Http.Api.Login', function () {
});

it('should fail with no username key in request body', function() {
return helper.request('https://localhost:8443')
return helper.request('https://localhost:9443')
.post('/login')
.send({password: "admin123"})
.expect(BAD_REQUEST_STATUS)
Expand All @@ -150,7 +150,7 @@ describe('Http.Api.Login', function () {
});

it('should fail with no password key in request body', function() {
return helper.request('https://localhost:8443')
return helper.request('https://localhost:9443')
.post('/login')
.send({username: "admin"})
.expect(BAD_REQUEST_STATUS)
Expand All @@ -164,7 +164,7 @@ describe('Http.Api.Login', function () {
// with credential in the header. Following test will fail if auth header
// is supported in the future, thus people will get alerted.
it('should fail with credential in request header', function() {
return helper.request('https://localhost:8443')
return helper.request('https://localhost:9443')
.post('/login')
.set('username', 'admin')
.set('password', 'admin123')
Expand All @@ -176,7 +176,7 @@ describe('Http.Api.Login', function () {
});

it('should fail no credential at all - https', function() {
return helper.request('https://localhost:8443')
return helper.request('https://localhost:9443')
.post('/login')
.expect(BAD_REQUEST_STATUS)
.expect(function(res) {
Expand Down Expand Up @@ -237,7 +237,7 @@ describe('Http.Api.Login', function () {
});

it('should fail with auth disabled', function() {
return helper.request('https://localhost:8443')
return helper.request('https://localhost:9443')
.post('/login')
.send({username: "admin", password: "admin123"})
.expect(NOT_FOUND_STATUS);
Expand All @@ -260,7 +260,7 @@ describe('Http.Api.Login', function () {
});

it('should fail with auth', function() {
return helper.request('https://localhost:8443')
return helper.request('https://localhost:9443')
.post('/login')
.send({username: "admin", password: "admin123"})
.expect(ERROR_STATUS)
Expand All @@ -285,7 +285,7 @@ describe('Http.Api.Login', function () {
});

it('should fail with auth', function() {
return helper.request('https://localhost:8443')
return helper.request('https://localhost:9443')
.post('/login')
.send({username: "admin", password: "admin123"})
.expect(UNAUTHORIZED_STATUS)
Expand Down
Loading