Support storing MDM bootstrap packages in S3 #21156
Conversation
|
|
||
| // TODO(mna): there is an inherent risk that we could delete files that were | ||
| // added between the query to list used IDs and now. We could minimize that | ||
| // risk by checking that the S3 file is older than a few seconds. |
There was a problem hiding this comment.
I think this is worth adding, I'll do it next week. It should make it practically impossible to delete S3 files that are in use (except in extreme cases where S3 API is super slow or the clock difference between S3 and our servers is significant, which I assume is very unlikely if Fleet is hosted in AWS itself).
There was a problem hiding this comment.
S3 has consistency guarantees:
Amazon S3 provides strong read-after-write consistency for PUT and DELETE requests of objects in your Amazon S3 bucket in all AWS Regions. This behavior applies to both writes to new objects as well as PUT requests that overwrite existing objects and DELETE requests. In addition, read operations on Amazon S3 Select, Amazon S3 access controls lists (ACLs), Amazon S3 Object Tags, and object metadata (for example, the HEAD object) are strongly consistent.
https://docs.aws.amazon.com/AmazonS3/latest/userguide/Welcome.html#ConsistencyModel
Not sure if it helps in this case, but figured I'd at least mention it.
There was a problem hiding this comment.
Thanks! Unfortunately in that case this doesn't change the risk because it's a question of integrity across mysql and S3 data - it's possible that an S3 object returned in that list is seen as "unused" by mysql and deleted, but in fact it should be seen as "used" because it has been added between the time we queried mysql and the time we list S3 objects.
| @@ -7119,7 +7119,7 @@ func (s *integrationMDMTestSuite) TestMDMEnabledAndConfigured() { | |||
| Token: uuid.New().String(), | |||
| Bytes: []byte("foo"), | |||
| Sha256: []byte("foo-sha256"), | |||
| })) | |||
| }, nil)) | |||
There was a problem hiding this comment.
I'll be working on adding an integration test suite, which enables S3 storage for all things that support it (software installers and bootstrap packages), so we have an easy way to integration-test those too (currently integration tests never use S3 for this).
There was a problem hiding this comment.
sounds great, thank you! if it can be used as inspiration, we have integration tests that use s3 (via minio) for the good old sandbox
fleet/server/service/integration_sandbox_test.go
Lines 36 to 45 in 1a0fe07
…rap-packages-in-s3
mna
requested review from
gillespi314,
lucasmrod,
getvictor and
a team
as code owners
| // cron job will eventually cleanup. | ||
| // 5. if everything succeeds, data is consistent and the S3 package | ||
| // cannot be used before it is uploaded (since the DB row is inserted | ||
| // after upload). |
There was a problem hiding this comment.
thanks for the comment, makes total sense
…rap-packages-in-s3
#19037
Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements)