Tests and improvements for secrets in cloud subdirectories #8791
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
radeusgd
added
the
CI: No changelog needed
radeusgd
changed the base branch from
develop
to
wip/radeusgd/8722-secrets-in-aws-cred
radeusgd
force-pushed
the
wip/radeusgd/8722-secrets-in-aws-cred
branch
from
5abfa9f to
850a054
Compare
radeusgd
force-pushed
the
wip/radeusgd/8723-secrets-in-folders
branch
from
b14189a to
7eb5a72
Compare
radeusgd
requested review from
jdunkerley,
GregoryTravis and
AdRiley
as code owners
radeusgd
commented
Jan 18, 2024
| @@ -1,3 +1,4 @@ | |||
| private | |||
There was a problem hiding this comment.
@JaroslavTulach per your suggestion on using the private keyword more, I thought that it may make sense to mark this module as private, so I did!
It only required moving as_hideable_value elsewhere (but I assume the new place actually fits it better anyway), because it needs to be usable in other libraries (e.g. AWS).
radeusgd
force-pushed
the
wip/radeusgd/8722-secrets-in-aws-cred
branch
from
850a054 to
d018d38
Compare
radeusgd
force-pushed
the
wip/radeusgd/8723-secrets-in-folders
branch
from
3929af5 to
ec3dffd
Compare
GregoryTravis
approved these changes
Jan 18, 2024
radeusgd
force-pushed
the
wip/radeusgd/8723-secrets-in-folders
branch
4 times, most recently
from
2c09f61 to
53d690d
Compare
Base automatically changed from
wip/radeusgd/8722-secrets-in-aws-cred
to
develop
January 19, 2024 19:00
mergify
bot
requested review from
4e6,
JaroslavTulach,
hubertp and
Akirathan
as code owners
radeusgd
force-pushed
the
wip/radeusgd/8723-secrets-in-folders
branch
from
b20faae to
c3c0a37
Compare
jdunkerley
approved these changes
Jan 23, 2024
… everything relevant is covered by Enso_User tests below
radeusgd
commented
Jan 23, 2024
Comment on lines
-18
to
-32
| Test.group "Enso Cloud Basic Utils" pending=setup.pending <| | ||
| Test.specify "will report Not_Logged_In if no credentials file is found" <| | ||
| non_existent_file = (enso_project.data / "nonexistent-file") . absolute . normalize | ||
| non_existent_file.exists.should_be_false | ||
|
|
||
| Test_Environment.unsafe_with_environment_override "ENSO_CLOUD_CREDENTIALS_FILE" non_existent_file.path <| | ||
| # This test has to run before any other Cloud access, otherwise the token may already be cached. | ||
| Cloud_Utils.authorization_header.should_fail_with Cloud_Utils.Not_Logged_In | ||
|
|
||
| Test.specify "should be able to get the cloud URL from environment" <| | ||
| api_url = Cloud_Utils.cloud_root_uri | ||
| api_url.should_equal setup.api_url.to_text | ||
|
|
||
| Test.specify "should be able to read the authorization token" pending=setup.mock_only_pending <| | ||
| Cloud_Utils.authorization_header.to_display_text.should_equal "Authorization: Bearer "+Cloud_Tests_Setup.test_token |
There was a problem hiding this comment.
I've removed these tests, because they were checking private implementation details (and I think we are not yet skipping private check for tests).
IMO it was OK to remove because everything relevant is covered by Enso_User tests below which should be run on CI because they only rely on the http-test-helper mock, which is available on CI.
So IMO there was not much added value in these whitebox tests.
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.
Pull Request Description
Enso_File.create_directoryandEnso_File.delete, and basic tests for itEnso_Secret.listis obtained - using a different Cloud endpoint allows us to implement the desired logic, the default endpoint was giving us all secrets which was not what we wanted here.Enso_Secret.updateand tests for itImportant Notes
Notes describing any problems with the current Cloud API:
https://docs.google.com/document/d/1x8RUt3KkwyhlxGux7XUGfOdtFSAZV3fI9lSSqQ3XsXk/edit
Apparently, everything that was needed to make this feature work has already been implemented, although a few features needed workarounds on Enso side to work properly.
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build.