search

SUSE / velum

Dashboard for CaaS Platform clusters (v1, v2 and v3)
https://www.suse.com/
Apache License 2.0
54 stars 30 forks source link

Allow uploading certificates via file #593

Closed bergmannf closed 6 years ago

bergmannf commented 6 years ago

Fixes bsc#1097753.

bergmannf commented 6 years ago

(3) M - Bug 1097753 - Certificate should be uploaded as a file

vitoravelino commented 6 years ago

Previously we started allowing the user to copy and paste because it was more convenient but now we are letting the user to select a file. In #590, we are only allowing the file selection and no copy and paste.

To maintain consistency regarding certificates/keys, should we change the workflow here to only file selection or change the #590 to also allow copy and paste from the user?

// cc @flavio @eotchi

flavio commented 6 years ago

To maintain consistency regarding certificates/keys, should we change the workflow here to only file selection or change the #590 to also allow copy and paste from the user?

I agree with that. Let's keep everything consistent, we got feedback from early users, they didn't like the copy&paste thing -> let's adapt and move to file upload only.

vitoravelino commented 6 years ago

I agree with that. Let's keep everything consistent, we got feedback from early users, they didn't like the copy&paste thing -> let's adapt and move to file upload only.

Based on this, could you adapt the PR to be similar to what we did on #590, @bergmannf? There are some screenshots on that PR but lemme know if you still need help to understand them.

MaximilianMeister commented 6 years ago

I agree with that. Let's keep everything consistent, we got feedback from early users, they didn't like the copy&paste thing -> let's adapt and move to file upload only.

agree on consistency. but we should make sure when we have upload only, that we also offer a download, otherwise it might be difficult for the user to see if the cert is the correct one

vitoravelino commented 6 years ago

agree on consistency. but we should make sure when we have upload only, that we also offer a download, otherwise it might be difficult for the user to see if the cert is the correct one

We have an input text that is disabled/readonly that shows what they've uploaded.

MaximilianMeister commented 6 years ago

We have an input text that is disabled/readonly that shows what they've uploaded.

oh right, that should be enough :+1:

bergmannf commented 6 years ago

I reworked this PR: instead of adding more and more fixture files, I now use the generated certificates for the new tests, the way they are used in the verification tests.

chentex commented 6 years ago

I test it and it did upload the certificate.

flavio commented 6 years ago

What is blocking us from merging this PR on master?