[BUG] Unable to create repository with SSH key authentication: "o is undefined"
Rancher Server Setup
- Rancher version: v2.8.1
- Installation option (Docker install/Helm Chart): Helm Chart
- If Helm Chart, Kubernetes Cluster and version (RKE1, RKE2, k3s, EKS, etc): RKE2
- Proxy/Cert Details: None
Information about the Cluster
- Kubernetes version:
v1.26.12+rke2r1 - Cluster Type (Local/Downstream): Local
User Information
- What is the role of the user logged in? Admin
Describe the bug
When I go to "Cluster Management > Advanced > Repositories" and try to add a new repo with a new SSH key for authentication, the creation fails and displays "o is undefined".
To Reproduce
- Go to "Cluster Management > Advanced > Repositories".
- Click "Create".
- Fill in repo info and choose "Create a new SSH Key Secret" under "Authentication".
- Fill in the public and private key info.
- Click "Create".
Result
o is undefined
Expected Result
Successful creation of the SSH key and adding the repo.
Screenshots
(relax, it's not my real key… 😉)
Additional context
I had the same issue on v2.7.6 and just upgraded to v2.8.1 hoping it would go away.
Thanks @lindhe, I could replicate it in v2.8.1 as well. The workaroud would be to create the repository with empty authentication, and then edit the YAML file to add them.
e2e test coverage LGTM however it might be worth testing this with legit SSH keys. moving to 'To Test' @izaac
We can test manually with a couple of encryption algos to be sure. RSA and other more modern.
If we can include ed25519 in the test, that would be good for me. :)
@izaac @yonasberhe23 @lindhe Thank you for reviewing the tests, I'm trying to understand why we would want to use real SSH keys or fake SSH key that follows the ed25519 standard? we're not verifying the active/successful state of the repo post-creation, we just check for the element to be there and then we delete the repo. Same applies for the basic auth case. Could you please provide further clarification?
@momesgin sure. As I understand the expected behavior in this case is to create a working repository.
UI -> API -> ssh client/library -> repo with key auth
As part of the dev fix validation checking the UI -> API I think it would suffice.
On the other hand E2E we should make sure this feature is actually working after the fix. This require to at least validate the authentication is successful. This will confirm the fix works.
We've ran into these ssh authentication issues in the past where configuring these repos using kubectl worked but not from the UI and this can be only noticed on the repository status.
Here is an example of an issue: https://github.com/rancher/dashboard/issues/7746
In this case I worked on this issue in the past and I had different behavior with different algorithms for some reason. And that was working from kubectl
The E2E of this scenario is not too complex but we need a couple ssh keys using like rsa and ed25519. They don't necessarily need to be those, but those are very popular.
In case this results in a backend issue after the E2E test we can open another for follow up on rancher/rancher
@momesgin sure. As I understand the expected behavior in this case is to create a working repository.
UI -> API -> ssh client/library -> repo with key authAs part of the dev fix validation checking the
UI -> APII think it would suffice.On the other hand E2E we should make sure this feature is actually working after the fix. This require to at least validate the authentication is successful. This will confirm the fix works.
We've ran into these ssh authentication issues in the past where configuring these repos using
kubectlworked but not from the UI and this can be only noticed on the repository status.Here is an example of an issue: #7746
In this case I worked on this issue in the past and I had different behavior with different algorithms for some reason. And that was working from
kubectlThe E2E of this scenario is not too complex but we need a couple ssh keys using like rsa and ed25519. They don't necessarily need to be those, but those are very popular. In case this results in a backend issue after the E2E test we can open another for follow up on
rancher/rancher
As I understand the bug is about the creation process being broken. Based on the current UX/UI design, the repo creation page allows the user to add any urls(e.g. https://github.com/your-company/charts.git), and for the SSH case only checks that the private key field is not empty(no further validation). Then it redirects the user to the list page, and if there's an issue with the created repo it'll be reflected on the UI by displaying the error status. So even if the repo is not working the UI still behaves correctly.
@izaac @yonasberhe23 @bmdepesa I feel the automated tests here cover the bug that has been fixed here and will prevent regression.
The ask seems to be to add e2e tests that validate the underlying backend functionality - that has always been there and without automated tests.
I agree that ultimately we would want tests for this as an overall feature - but for this bug fix, it feels like the tests should be enough to not require manual test
@nwmac agreed. I will add a QA task to the backlog for tracking
@nwmac let's bring this up on the next sync up. We should be covering E2E like on provisioning.
Originally tests were only using Custer objects in the UI for Imported and Custom types. But now we have those with real clusters.
This should be the case for all features.