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.

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

[WIP] Move ConfigurationScriptSource#authentication to GitRepository #394

Conversation

Fryguy
Copy link
Member

@Fryguy Fryguy commented Jul 23, 2019

This is an attempt to deal with the "double ownership problem" with ConfigurationScriptSource#authentication and GitRepository. The idea is that in the model we can delegate authentication stuff to the GitRepository. Thus, this migration first ensures that a git_repository exists, then moves the authentication over to it.

In GitRepository model, while it might have it's own authentication via AuthenticationMixin, we also add a separate belongs_to :authentication which is a referenced authentication. This way we can support both directly owned authentications and not-owned authentications.

cc @gtanzillo @carbonin @agrare @NickLaMuro

@Fryguy
Copy link
Member Author

Fryguy commented Jul 23, 2019

Missing specs, but wanted to just get it out there.

@miq-bot miq-bot added the wip label Jul 23, 2019
@miq-bot
Copy link
Member

miq-bot commented Jul 23, 2019

Checked commit Fryguy@a09a688 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 4 offenses detected

db/migrate/20190723023241_move_configuration_script_source_authentication_to_git_repository.rb

@carbonin
Copy link
Member

Won't we lose the SCM credentials for existing ConfigurationScriptSources with this change? Should this also create GitRepository instances in the case that we don't have one currently? I don't see the authentication getting moved over in the create step in ManageIQ/manageiq#19027.

@Fryguy Fryguy closed this Jul 23, 2019
@Fryguy Fryguy deleted the move_configuration_script_source_authentication_to_git_repository branch July 23, 2019 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants