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

Adjusted DDF schema and credentials validation #385

Merged
merged 5 commits into from
Aug 6, 2020

Conversation

skateman
Copy link
Member

@skateman skateman commented Jun 29, 2020

In order to make this form work, I had to do some tradeoffs:

  • The default_value_for in the ContainerManagerMixin prevents the usage of create_from_params so I deleted it.
  • We are (kinda) using the same authentication for each endpoint (except kubevirt) by copying the same authentication with different authtypes. I solved this via redefining the .create_from_params with the replication of the same data among endpoints. However, I'd rather see the provider using just the single authentication instead.
  • As we never send down passwords to the client and the token authentication has no any public value that we could send down, we end up with not submitting anything in the authentications if there's no token change. This can cause all authentications to be deleted upon an edit without a token change. In order to avoid this, I redefined the #edit_with_params to make sure that all authentications exist for all the endpoints. Again, this would be much easier if we could just use a single auth instead of copying the existing one N times.
  • The endpoint for the kubevirt provider is the same as the default one, but it uses a different token. On the old forms this was sorted out by copying the data on the form level, which is not possible via DDF. For now the user has to fill these fields manually, but I hope that the Detect button will be able to copy these values on-demand.
  • The Detect button doesn't work yet, there's no API endpoint for it and there's a styling issue that I'm trying to fix
  • Validation is only working for the default endpoint and partially for kubevirt. The other endpoints need to have context-free methods than can work with the params I am sending. Kubevirt is using the same validation as the default endpoint, but it doesn't actually detect if the given token allows access to the kubevirt features.

Parent issue: ManageIQ/manageiq#18818

@skateman skateman force-pushed the ddf-forms-adjust branch 3 times, most recently from 111bce6 to 59c2dbd Compare July 31, 2020 13:06

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@agrare agrare force-pushed the ddf-forms-adjust branch from 2edc625 to b54260e Compare August 3, 2020 14:47
@skateman skateman changed the title [WIP] Adjusted DDF schema and credentials validation Adjusted DDF schema and credentials validation Aug 6, 2020
@miq-bot miq-bot removed the wip label Aug 6, 2020
@agrare agrare force-pushed the ddf-forms-adjust branch from 91a10d4 to 6391c54 Compare August 6, 2020 12:44
@miq-bot
Copy link
Member

miq-bot commented Aug 6, 2020

Checked commits skateman/manageiq-providers-kubernetes@3e67e19~...6391c54 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@skateman
Copy link
Member Author

skateman commented Aug 6, 2020

I guess this is good to go then 👍

@agrare
Copy link
Member

agrare commented Aug 6, 2020

@gtanzillo can you review since I have some commits in here now?

@gtanzillo gtanzillo self-assigned this Aug 6, 2020
@gtanzillo gtanzillo merged commit 563ade0 into ManageIQ:master Aug 6, 2020
@skateman skateman deleted the ddf-forms-adjust branch August 6, 2020 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants