-
Notifications
You must be signed in to change notification settings - Fork 248
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
[RFC] schema: add storage.encryption section #515
Conversation
dc8f0ab
to
7f708b5
Compare
/cc @dgonyeo for review. What do you use to keep JSON format coherent? I used |
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.
File headers should be 2018.
The docs should probably also have a section in the operator notes on how wipeVolume
is handled with regards to PXE booting with persistant root (similar to the filesystems section).
Will this be ammended with the implementation or is that coming in a seperate PR?
config/types/encryption.go
Outdated
// Validate ensures a Cryptsetup entry is sane | ||
// | ||
// It fulfills validate.validator interface. | ||
func (e Encryption) Validate() report.Report { |
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.
These can be broken up into Validate<FIELDNAME>()
functions which will let the validator logic be a little more precise with line numbers when reporting errors. The file section has some examples.
7f708b5
to
f835af2
Compare
Can one of the admins verify this patch? |
This introduces a new `storage.encryption` section which can be used to provision LUKS volumes. Volumes provisioned this way are meant to be unlocked early in the boot process and made available in the initramfs (e.g. the rootfs).
f835af2
to
74e3aa1
Compare
@ajeddeloh thanks, I didn't know about generics Implementation will follow in a separate PR, with the key fetching/unwrapping logic in coreos/coreos-cryptagent#5 (in case you have review cycles for that too, feel free to). |
} | ||
|
||
return r | ||
} |
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.
You should add a func (c Content) ValidateSource() report.Report
that calls validateUrl
(or maybe something else since this only supports https?) on c.Source
.
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.
I've slightly bent validateURL
to take an additional optional subset of whitelisted schemes, let me know if you prefer something simpler and dedicated instead.
74e3aa1
to
340e651
Compare
@arithx is there anything you want to salvage from here? Otherwise I'd just close it as just another stale PR. |
@lucab feel free to nuke. |
This introduces a new
storage.encryption
section which can be usedto provision LUKS volumes.
Volumes provisioned this way are meant to be unlocked early in the
boot process and made available in the initramfs (e.g. the rootfs).