Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(windows-agent): Hide unnecessarily exposed fields in the registry #369

Merged
merged 23 commits into from
Oct 30, 2023
Merged

feat(windows-agent): Hide unnecessarily exposed fields in the registry #369

merged 23 commits into from
Oct 30, 2023

Conversation

EduardGomezEscandell
Copy link
Contributor

@EduardGomezEscandell EduardGomezEscandell commented Oct 26, 2023 •
edited
Loading

The registry exposes a lot of fields that should not be exposed to the user. It should really only show:

  • Organization-provided pro token
  • Organization-provided landscape config

The rest should be moved to a file. A side benefit is that the registry becomes (almost) read-only to us, meaning we can rethink our disabled virtualization.

UDENG-1685

@EduardGomezEscandell EduardGomezEscandell changed the base branch from main to hostagent-url-in-file October 26, 2023 14:51
@EduardGomezEscandell EduardGomezEscandell marked this pull request as draft October 26, 2023 14:51
@EduardGomezEscandell EduardGomezEscandell self-assigned this Oct 26, 2023
@EduardGomezEscandell EduardGomezEscandell force-pushed the config-disk-storage branch 2 times, most recently from 47a3fca to 1209f1e Compare October 26, 2023 16:23
Base automatically changed from hostagent-url-in-file to main October 27, 2023 08:26
@EduardGomezEscandell EduardGomezEscandell marked this pull request as ready for review October 27, 2023 08:44
@EduardGomezEscandell EduardGomezEscandell force-pushed the config-disk-storage branch 4 times, most recently from 1fb9d48 to 91bc677 Compare October 27, 2023 09:57
CarlosNihelton
CarlosNihelton requested changes Oct 27, 2023
View reviewed changes
Copy link
Contributor

@CarlosNihelton CarlosNihelton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing big catches my eyes, but I saw some interesting corner cases and a few nitpicks.

windows-agent/internal/config/config_layering.go Outdated Show resolved Hide resolved
windows-agent/internal/config/config_layering.go Outdated Show resolved Hide resolved
windows-agent/internal/config/config_layering.go Outdated Show resolved Hide resolved
windows-agent/internal/config/config_layering.go Outdated Show resolved Hide resolved
windows-agent/internal/config/config_layering.go Outdated Show resolved Hide resolved
windows-agent/internal/config/config_marshal.go Outdated Show resolved Hide resolved
windows-agent/internal/config/config_marshal.go Outdated Show resolved Hide resolved
windows-agent/internal/config/config_marshal.go Outdated Show resolved Hide resolved
windows-agent/internal/config/config.go Show resolved Hide resolved
windows-agent/internal/config/config.go Show resolved Hide resolved
@EduardGomezEscandell
Config: Rename config.SubsriptionSource to config.Source
3e647a0 
This allows the enum to be used for Landscape as well
@EduardGomezEscandell
Config: Implement layering system
e2deabb
@EduardGomezEscandell
Config: Use layering system
8231e1c
@EduardGomezEscandell
Config: Implement source-wise subscription getter and setter
ad3b5cb
@EduardGomezEscandell
Config: Rewrite config load/dump
c156e4c 
Now we split the settings between file and registry
@EduardGomezEscandell
Config: Rewrite checksum validator
a932d13 
It becomes much simpler now that the checksum is just another setting
@EduardGomezEscandell
Adapt callers to new config function signatures
73b3c3a
@EduardGomezEscandell
Update documentation
2aa9fef
@EduardGomezEscandell
Adapt tests to new Config
e0cb976
@EduardGomezEscandell
Config: fix log messages refering to the registry
c71b0fa
@EduardGomezEscandell
Rename GUI-sourced config to User-sourced
b30fffb 
Both for the subscription and landscape
@EduardGomezEscandell
Rename config_layering.go to config_source.go
cb552bc
@EduardGomezEscandell
Clarify semantics of hasChanged
dc6accf
@EduardGomezEscandell
Code formatting in config_marshal.go
7c2dbf0
@EduardGomezEscandell
Update docstring for config.LandscapeClientConfig
a75f102
@EduardGomezEscandell
Copy link
Contributor Author

Rebased main accidentally. Real changes start at Rename GUI-sourced config to User-sourced

CarlosNihelton
CarlosNihelton approved these changes Oct 30, 2023
View reviewed changes
Copy link
Contributor

@CarlosNihelton CarlosNihelton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍🏽

@EduardGomezEscandell EduardGomezEscandell merged commit 79acfc8 into main Oct 30, 2023
33 of 34 checks passed
@EduardGomezEscandell EduardGomezEscandell deleted the config-disk-storage branch October 30, 2023 07:42
@EduardGomezEscandell EduardGomezEscandell mentioned this pull request Oct 30, 2023
Merged
EduardGomezEscandell added a commit that referenced this pull request Nov 14, 2023
@EduardGomezEscandell
Fix Ubuntu Pro Token registry key in E2E tests
8a9f8d9 
This regression was introduced in PR #369
@EduardGomezEscandell EduardGomezEscandell mentioned this pull request Nov 14, 2023
Merged
EduardGomezEscandell added a commit that referenced this pull request Nov 14, 2023
@EduardGomezEscandell
fix(end-to-end): Fix Ubuntu Pro Token registry key used in E2E tests (#…
104e2e9 
…400)

The key value in which the Ubuntu Pro token is stored was changed in PR
#369 from `ProTokenOrg` to `UbuntuProToken`.

This error went unnoticed because the E2E tests were not working until
#397.
EduardGomezEscandell added a commit that referenced this pull request Nov 20, 2023
@EduardGomezEscandell
feat(windows-agent): Remove all writes to registry, and remove virtua…
972ad3e 
…lization exception (#374)

With the recent changes in #369, we no longer need to write to the
registry.

There is one behavior change here: setting the registry as read-only no
longer has any effect on the program. Until now, it was an implicit
message from the administrator to UP4W to block attempts to modify the
config.


----

UDENG-1696
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CarlosNihelton CarlosNihelton CarlosNihelton approved these changes

@didrocks didrocks Awaiting requested review from didrocks

Assignees

@EduardGomezEscandell EduardGomezEscandell

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@EduardGomezEscandell @CarlosNihelton