Skip to content

Conversation

@SkorikSergey
Copy link
Contributor

What does this PR do?

Support SSH keys from environment variables in SshUrlNoOauthPatFactory E2E test:

  • Added addSshKeysFromStrings() method to UserPreferences page object for direct SSH key string input
  • Added TS_SELENIUM_SSH_PRIVATE_KEY and TS_SELENIUM_SSH_PUBLIC_KEY constants to FACTORY_TEST_CONSTANTS
  • Updated SshUrlNoOauthPatFactory test to support dual-mode SSH key loading:
    • Priority 1: Environment variables (if set)
    • Priority 2: File paths (fallback)
  • Added input validation for SSH keys with clear error messages

Screenshot/screencast of this PR

What issues does this PR fix or reference?

This PR is part of https://issues.redhat.com/browse/CRW-5392

How to test this PR?

To test with environment variables set TS_SELENIUM_SSH_PRIVATE_KEY and TS_SELENIUM_SSH_PUBLIC_KEY
To test with file paths set TS_SELENIUM_SSH_PRIVATE_KEY_PATH and TS_SELENIUM_SSH_PUBLIC_KEY_PATH

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

@SkorikSergey SkorikSergey self-assigned this Oct 28, 2025
@SkorikSergey SkorikSergey added the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Oct 28, 2025
await userPreferences.addSshKeysFromStrings(privateSshKey, publicSshKey);
} else {
Logger.info('Using SSH keys from file paths');
await userPreferences.addSshKeys(privateSshKeyPath, publicSshKeyPath);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest renaming userPreferences.addSshKeys() to userPreferences.uploadSshKeys() to make the difference with userPreferences.addSshKeysFromStrings() obvious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

userPreferences already has uploadSshKeys() that used in addSshKeys()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about addSshKeysFromFiles()?

Copy link
Contributor

@dmytro-ndp dmytro-ndp Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addSshKeysFromFiles() sounds good as well to me

Thank you!

Copy link
Contributor

@dmytro-ndp dmytro-ndp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to merge, with a minor non-critical notice.

@SkorikSergey SkorikSergey marked this pull request as ready for review October 29, 2025 14:51
@SkorikSergey SkorikSergey merged commit 83f7499 into main Oct 29, 2025
6 checks passed
@SkorikSergey SkorikSergey deleted the getSshKeysByEnv branch October 29, 2025 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/qe status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants