NC | Secret Keys Encryption Gaps #7979
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
romayalon
force-pushed
the
romy-nc-secret-encryption
branch
4 times, most recently
from
0e4cc15 to
912987c
Compare
romayalon
force-pushed
the
romy-nc-secret-encryption
branch
2 times, most recently
from
bce0de4 to
561f77c
Compare
romayalon
force-pushed
the
romy-nc-secret-encryption
branch
from
561f77c to
9508d82
Compare
guymguym
reviewed
Apr 22, 2024
| @@ -95,6 +95,7 @@ async function main(argv = minimist(process.argv.slice(2))) { | |||
| const user_input = user_input_from_file || argv; | |||
| config_root = argv.config_root ? String(argv.config_root) : config.NSFS_NC_CONF_DIR; | |||
| if (!config_root) throw_cli_error(ManageCLIError.MissingConfigDirPath); | |||
| if (argv.config_root) config.NSFS_NC_CONF_DIR = String(argv.config_root); | |||
There was a problem hiding this comment.
the argv.config_root?
I would love to remove it, it's very annoying
There was a problem hiding this comment.
so why did you add it in this PR? is it needed for any case?
There was a problem hiding this comment.
I meant to the option of passing config_root,
I added it because if we won't update it the master_keys.json will get written to /etc/noobaa.conf.d/
There was a problem hiding this comment.
why would it be created always in etc? our config.js code already handles config_dir_redirect no?
There was a problem hiding this comment.
yes, but this was initially introduced before we had anything, and for cases when there is not config_dir_redirect.
I think we use it only for tests
romayalon
force-pushed
the
romy-nc-secret-encryption
branch
from
1605f3a to
bc54fe2
Compare
guymguym
approved these changes
Apr 22, 2024
Signed-off-by: Romy <35330373+romayalon@users.noreply.github.com>
romayalon
force-pushed
the
romy-nc-secret-encryption
branch
from
1a24129 to
477ef92
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explain the changes
Master Key Manager -
Config.js -
Checked for the correctness of the master keys configuration.
On executable flow - Checked that the executable files exist and can be executed.
Unit Tests additions -
Issues: Fixed #xxx / Gap #xxx
Testing Instructions:
sudo jest --testRegex=jest_tests/test_nc_master_keys_execsudo jest --testRegex=jest_tests/test_nc_nsfs_config_schema