Conversation
Signed-off-by: Mayursinh Sarvaiya <marvinduff97@gmail.com>
Marvin9
added
area/ui
✅ Deploy Preview for docs-kargo-io ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5404 +/- ##
==========================================
- Coverage 56.10% 56.09% -0.01%
==========================================
Files 411 411
Lines 30063 30077 +14
==========================================
+ Hits 16868 16873 +5
- Misses 12221 12227 +6
- Partials 974 977 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| string username = 6; | ||
| // password is the password or token for authentication. | ||
| string password = 7; | ||
| string ssh_private_key = 9; |
There was a problem hiding this comment.
We also need to eventually support GitHub App Authentication. Which has the fields:
githubAppClientIDgithubAppPrivateKeygithubAppInstallationID
Is continually expanding CreateCredentialsRequest API the sustainable approach to this?
Should UI switch to the Apply endpoint and just create Secret resources with baked-in understanding of the various fields (username/password/privatekey/githubXXXX/etc...)? Then the ConnectRPC API layer never needs to updated.
There was a problem hiding this comment.
Oh. I thought its only sshPrivateKey. You are right, we should start using generic secrets.
Are you suggesting is to remove CreateCredentials approach? even for normal credentials?
There was a problem hiding this comment.
There was a problem hiding this comment.
@jessesuen I had a plan already. It's written down in some issue somewhere. I just need to find it. Credentials come in all different shapes. What I want to do is have Secret templates for all different kinds of creds and you fill in the blanks for whatever type of cred like you're doing a Mad Lib. Adding support for a new kind of credential would be as easy as providing a new template. In the event that a template does not exist for some sort of credential, the option would remain to just give us the cred type and data block.
But, imho, that more comprehensive approach to UI/CLI support for different kinds of credentials ought not to block this ssh-key-specific incremental improvement.
Having said that, I've reached the point of discouraging people from using SSH with Kargo for the simple reason that none of the providers' APIs accept ssh keys as credentials. So when you choose to use ssh creds for basic git operations like cloning, pushing, etc., you force yourself into having to maintain a second set of creds for the same repo(s) so that you can open and merge PRs, etc.
Let's chat about it face to face in the coming week.
There was a problem hiding this comment.
@krancour are we going with this or should close PR for now?
|
@Marvin9 I think @jessesuen should make this call. We had a conversation yesterday about deprecating ssh support in v1.10.0 and maybe removing it many releases from now. Probably no sooner than v1.14.0, if ever. So I'm uncertain whether we want to introduce new ssh-related features. Small tangent, which I've mentioned before: I think how we define creds, in general, is in need of an overhaul, which metadata of some sort describing the different "shapes" of creds so that the UI and CLI alike can better accommodate the many creds that aren't simply username and password. I'm going to put together what I'll call a "creds v2" proposal, but this is a long-term thing. In the meantime, let's have @jessesuen's make the call on whether to include this feature or not. |
|
Closing in favor of #5978 |
3 participants
#5149