add an endpoint to get an aggregate summary of bootstrap packages #11156

roperzh · 2023-04-11T22:31:33Z

Related to #10934

Checklist for submitter

If some of the following don't apply, delete the relevant line.

Documented any API changes (docs/Using-Fleet/REST-API.md or docs/Contributing/API-for-contributors.md)
Input data is properly validated, SELECT * is avoided, SQL injection is prevented (using placeholders for values in statements)
Added/updated tests
Manual QA for all new/changed functionality

roperzh · 2023-04-11T22:33:40Z

server/datastore/mysql/migrations/tables/20230411102858_CreateHostBootstrapPackagesTable.go

+          CREATE TABLE host_mdm_apple_bootstrap_packages (
+	    host_uuid    varchar(127) NOT NULL,
+            command_uuid varchar(127) NOT NULL,
+
+            PRIMARY KEY (host_uuid, command_uuid),
+            FOREIGN KEY (command_uuid) REFERENCES nano_commands (command_uuid) ON DELETE CASCADE
+          )`)


@mna @gillespi314 I'm not sure if this table is the right approach, do we want a more general "fleet commands" table? this table would still need a column with a flag/enum we can use to distinguish which commands are related to bootstrap packages.

up to you!

Hmm yeah good question, this is the only command that we need this for at the moment, right? The disk encryption profile is also treated a bit as a special case, but it does not need something like this AFAIR? Given that this is the first need for this, it could make sense to use it like you proposed, and rename it to something more general if and when we need something similar for other "special commands"?

server/service/integration_mdm_test.go

docs/Using-Fleet/REST-API.md

mna · 2023-04-12T19:45:58Z

docs/Using-Fleet/REST-API.md

+  "name": "bootstrap-package.pkg",
+  "team_id": 0,
+  "sha256": "6bebb4433322fd52837de9e4787de534b4089ac645b0692dfb74d000438da4a3",
+  "token": "AA598E2A-7952-46E3-B89D-526D45F7E233"


Just curious what's token in this response? Is that extracted from the package?

Maybe it would be worth adding an explanation of token and sha256 (the hash of the package, I presume).

mna · 2023-04-12T19:49:32Z

docs/Using-Fleet/REST-API.md

+
+Download a bootstrap package.
+
+`GET /api/v1/fleet/mdm/apple/bootstrap`


Given the path structure (GET /mdm/apple/bootstrap/{team_id}/metadata returns the metadata for the bootstrap package of that team, DELETE /mdm/apple/bootstrap/{team_id} deletes the package for that team) I would expect GET /mdm/apple/bootstrap/{team_id} to download the package for a given team? There may be a technical limitation that prevents us from doing this/requiring to use a token approach instead?

deletes the package for that team) I would expect GET /mdm/apple/bootstrap/{team_id} to download the package for a given team? There may be a technical limitation that prevents us from doing this/requiring to use a token approach instead?

yes! the bootstrap package needs to be accessed without user authentication, so it's gated behind the token.

having said that, I think we could have used both? the team_id is redundant but looks more consistent: GET /mdm/apple/bootstrap/{team_id}?token={token}

we have already released that endpoint, but we're doing other breaking changes, so maybe we can add this to the list. I'll make a note to discuss with Luke when he's back.

mna · 2023-04-12T19:50:27Z

docs/Using-Fleet/REST-API.md

+
+The summary can optionally be filtered by team id.
+
+`GET /api/v1/fleet/mdm/apple/bootstrap/summary`


Similar to the previous comment, could that be GET /mdm/apple/bootstrap/{team_id}/summary for consistency?

It makes sense, I think it was specified that way to be consistent with all the other /resource_here/summary endpoints which take a team_id as a query parameter. I'll also add this to the list to double check with Luke when he's back.

FWIW, I think the trend on the frontend at least has been toward using team id as a query param rather than a route param for entities that are team-related (but not teams themselves).

mna · 2023-04-12T20:06:03Z

server/datastore/mysql/migrations/tables/20230411102858_CreateHostBootstrapPackagesTable.go

+          CREATE TABLE host_mdm_apple_bootstrap_packages (
+	    host_uuid    varchar(127) NOT NULL,
+            command_uuid varchar(127) NOT NULL,
+
+            PRIMARY KEY (host_uuid, command_uuid),
+            FOREIGN KEY (command_uuid) REFERENCES nano_commands (command_uuid) ON DELETE CASCADE
+          )`)


Hmm yeah good question, this is the only command that we need this for at the moment, right? The disk encryption profile is also treated a bit as a special case, but it does not need something like this AFAIR? Given that this is the first need for this, it could make sense to use it like you proposed, and rename it to something more general if and when we need something similar for other "special commands"?

server/fleet/datastore.go

server/fleet/service.go

server/datastore/mysql/apple_mdm.go

server/service/integration_mdm_test.go

docs/Using-Fleet/REST-API.md

Co-authored-by: Martin Angers <martin.n.angers@gmail.com>

codecov · 2023-04-13T15:54:28Z

Codecov Report

Patch coverage: 27.38% and project coverage change: -0.57 ⚠️

Comparison is base (ddb5894) 60.92% compared to head (56f0665) 60.35%.

❗ Current head 56f0665 differs from pull request most recent head 6f9fa95. Consider uploading reports for the commit 6f9fa95 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #11156      +/-   ##
==========================================
- Coverage   60.92%   60.35%   -0.57%     
==========================================
  Files         525      529       +4     
  Lines       54573    55360     +787     
==========================================
+ Hits        33246    33411     +165     
- Misses      18369    18962     +593     
- Partials     2958     2987      +29

Impacted Files	Coverage Δ
ee/server/service/mdm.go	`24.17% <0.00%> (+13.78%)`	⬆️
server/fleet/apple_mdm.go	`40.38% <ø> (ø)`
server/fleet/datastore.go	`0.00% <ø> (ø)`
server/datastore/mysql/apple_mdm.go	`70.85% <7.14%> (-2.77%)`	⬇️
server/service/apple_mdm.go	`58.35% <36.84%> (-1.84%)`	⬇️
...20230411102858_CreateHostBootstrapPackagesTable.go	`72.22% <72.22%> (ø)`
server/service/handler.go	`87.08% <100.00%> (+0.03%)`	⬆️

... and 26 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

roperzh · 2023-04-20T20:38:37Z

server/mdm/apple/util.go

@@ -141,3 +143,15 @@ func secureRandInt(min, max int64) (int, error) {
 	}
 	return random, nil
 }
+
+func FmtErrorChain(chain []mdm.ErrorChain) string {


this seems to be the exact same function as *MDMAppleCheckinAndCommandService.fmtErrorChain, can we completely replace the struct function in favor of this?

roperzh · 2023-04-21T17:56:14Z

LGTM!

add an endpoint to get an aggregate summary of bootstrap packages

0781e88

roperzh requested review from ksatter, lukeheath and a team as code owners April 11, 2023 22:31

roperzh temporarily deployed to Docker Hub April 11, 2023 22:31 — with GitHub Actions Inactive

roperzh mentioned this pull request Apr 11, 2023

add API docs for bootstrap pacakge endpoints #11125

Closed

1 task

roperzh commented Apr 11, 2023

View reviewed changes

server/service/integration_mdm_test.go Outdated Show resolved Hide resolved

roperzh added 2 commits April 11, 2023 20:11

add missing mock

4fe63d3

Merge remote-tracking branch 'origin/main' into 10934-bp-endpoints

e2925ed

roperzh temporarily deployed to Docker Hub April 11, 2023 23:16 — with GitHub Actions Inactive

mna reviewed Apr 12, 2023

View reviewed changes

ghernandez345 reviewed Apr 13, 2023

View reviewed changes

docs/Using-Fleet/REST-API.md Outdated Show resolved Hide resolved

Apply suggestions from code review

56f0665

Co-authored-by: Martin Angers <martin.n.angers@gmail.com>

gillespi314 temporarily deployed to Docker Hub April 13, 2023 15:36 — with GitHub Actions Inactive

Address comments

1876aef

gillespi314 temporarily deployed to Docker Hub April 13, 2023 19:09 — with GitHub Actions Inactive

Check for nil command

c6450ef

gillespi314 temporarily deployed to Docker Hub April 13, 2023 20:07 — with GitHub Actions Inactive

gillespi314 added 5 commits April 13, 2023 15:19

Merge branch 'main' into 10934-bp-endpoints

206f5d3

Add bootstrap to list host filter and host detail

8c283e8

Merge branch 'main' into 10934-bp-endpoints

1178b74

Update labels with filter; integration test

f45cb9b

Cleanup comments

ae6cadd

gillespi314 temporarily deployed to Docker Hub April 15, 2023 16:20 — with GitHub Actions Inactive

roperzh temporarily deployed to Docker Hub April 20, 2023 13:09 — with GitHub Actions Inactive

gillespi314 mentioned this pull request Apr 20, 2023

Bootstrap package URL is missing #11248

Closed

Address comments

39a0efb

gillespi314 temporarily deployed to Docker Hub April 20, 2023 20:31 — with GitHub Actions Inactive

roperzh commented Apr 20, 2023

View reviewed changes

Fix json response; update test

e931a57

gillespi314 temporarily deployed to Docker Hub April 21, 2023 14:35 — with GitHub Actions Inactive

Refactor FmtErrorChain

3165b99

gillespi314 temporarily deployed to Docker Hub April 21, 2023 14:59 — with GitHub Actions Inactive

gillespi314 had a problem deploying to Docker Hub April 21, 2023 14:59 — with GitHub Actions Error

Merge branch 'main' into 10934-bp-endpoints

b057c2e

gillespi314 temporarily deployed to Docker Hub April 21, 2023 15:01 — with GitHub Actions Inactive

gillespi314 added 2 commits April 21, 2023 13:06

Merge branch 'main' into 10934-bp-endpoints

f55ea5e

Fix test

5368d41

gillespi314 temporarily deployed to Docker Hub April 21, 2023 18:11 — with GitHub Actions Inactive

Merge branch 'main' into 10934-bp-endpoints

e73cbee

gillespi314 temporarily deployed to Docker Hub April 21, 2023 20:02 — with GitHub Actions Inactive

Merge branch 'main' into 10934-bp-endpoints

6f9fa95

gillespi314 temporarily deployed to Docker Hub April 21, 2023 21:25 — with GitHub Actions Inactive

gillespi314 approved these changes Apr 22, 2023

View reviewed changes

gillespi314 merged commit 5c48789 into main Apr 22, 2023

gillespi314 deleted the 10934-bp-endpoints branch April 22, 2023 15:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add an endpoint to get an aggregate summary of bootstrap packages #11156

add an endpoint to get an aggregate summary of bootstrap packages #11156

roperzh commented Apr 11, 2023 •

edited by gillespi314

Loading

roperzh Apr 11, 2023

mna Apr 12, 2023

mna Apr 12, 2023

mna Apr 12, 2023

mna Apr 12, 2023

roperzh Apr 20, 2023

mna Apr 12, 2023

roperzh Apr 20, 2023

gillespi314 Apr 20, 2023

mna Apr 12, 2023

codecov bot commented Apr 13, 2023 •

edited

Loading

roperzh Apr 20, 2023

roperzh commented Apr 21, 2023


		Download a bootstrap package.

		`GET /api/v1/fleet/mdm/apple/bootstrap`


		The summary can optionally be filtered by team id.

		`GET /api/v1/fleet/mdm/apple/bootstrap/summary`

add an endpoint to get an aggregate summary of bootstrap packages #11156

add an endpoint to get an aggregate summary of bootstrap packages #11156

Conversation

roperzh commented Apr 11, 2023 • edited by gillespi314 Loading

Checklist for submitter

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 13, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

roperzh commented Apr 21, 2023

roperzh commented Apr 11, 2023 •

edited by gillespi314

Loading

codecov bot commented Apr 13, 2023 •

edited

Loading