Skip to content

Make trilead-api as optional as it contains some algo users may no want to use#199

Merged
jglick merged 1 commit intojenkinsci:masterfrom
olamy:trilead-api-optional
Mar 19, 2024
Merged

Make trilead-api as optional as it contains some algo users may no want to use#199
jglick merged 1 commit intojenkinsci:masterfrom
olamy:trilead-api-optional

Conversation

@olamy
Copy link
Member

@olamy olamy commented Mar 15, 2024

Signed-off-by: Olivier Lamy olamy@apache.org

Testing done

### Submitter checklist
- [ ] Make sure you are opening from a **topic/feature/bugfix branch** (right side) and not your main branch!
- [ ] Ensure that the pull request title represents the desired changelog entry
- [ ] Please describe what you did
- [ ] Link to relevant issues in GitHub or Jira
- [ ] Link to relevant pull requests, esp. upstream and downstream changes
- [ ] Ensure you have provided tests - that demonstrates feature works or fixes the issue

…nt to use

Signed-off-by: Olivier Lamy <olamy@apache.org>
@olamy olamy marked this pull request as ready for review March 19, 2024 00:40
@olamy olamy requested a review from a team as a code owner March 19, 2024 00:40
Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

And conversely I see that mina-sshd-api-core lists ssh-credentials as an optional dep. (It uses @Extension(optional = true) which would better be switched to @OptionalExtension as here, to avoid warnings on startup in the unlikely case ssh-credentials is not in fact enabled.)

@jglick jglick merged commit 7fcbaef into jenkinsci:master Mar 19, 2024
MarkEWaite added a commit to MarkEWaite/bom that referenced this pull request Mar 23, 2024
…enkinsci#3039)"

jenkinsci/ssh-credentials-plugin#199 makes
the trilead-api optional.  Likely needs some tuning to allow plugin
compatibility tests to pass.

This reverts commit 3fb5748.
MarkEWaite added a commit to jenkinsci/bom that referenced this pull request Mar 23, 2024
…3039)" (#3049)

jenkinsci/ssh-credentials-plugin#199 makes
the trilead-api optional.  Likely needs some tuning to allow plugin
compatibility tests to pass.

This reverts commit 3fb5748.
MarkEWaite added a commit to MarkEWaite/bom that referenced this pull request Mar 25, 2024
jenkinsci/ssh-credentials-plugin#199 makes the
trilead-api plugin an optional dependency of the ssh-credentials plugin.
That exposed a latent issue in the subversion plugin that is resolved
in jenkinsci/subversion-plugin#284 .

jenkinsci#3050 describes the details of
the test failures in the plugin bill of mateerials.  The failing tests
can be seen with the commands:

 PLUGINS=subversion TEST=CompareAgainstBaselineCallableTest bash local-test.sh
 PLUGINS=mina-sshd-api-core LINE=weekly bash local-test.sh

No need to block later releases, since the pull request thet fixes the
issue has been submitted and is ready for review.
@Dohbedoh
Copy link
Contributor

Dohbedoh commented May 2, 2024

Shouldn't the trilead authenticator implementation eventually be moved to trilead-api plugin ? Like it is the case for Jsch, see #152, and mina-sshd.

@olamy olamy deleted the trilead-api-optional branch May 3, 2024 00:19
@olamy
Copy link
Member Author

olamy commented May 3, 2024

Shouldn't the trilead authenticator implementation eventually be moved to trilead-api plugin ? Like it is the case for Jsch, see #152, and mina-sshd.

@Dohbedoh very good idea. Do you have time to take care of this? If not, let me know and I will.

@Dohbedoh
Copy link
Contributor

Dohbedoh commented May 8, 2024

I can take a look. But will need some coordination / release plan between those 2 plugins to avoid circular dependencies.

@olamy
Copy link
Member Author

olamy commented May 8, 2024

I can take a look. But will need some coordination / release plan between those 2 plugins to avoid circular dependencies.

sounds good. We can coordinate at the pub :)

@basil
Copy link
Member

basil commented May 14, 2024

After this PR, in the context of https://github.com/jenkinsci/acceptance-test-harness

import com.cloudbees.jenkins.plugins.sshcredentials.SSHAuthenticatorFactory
ExtensionList.lookup(SSHAuthenticatorFactory.class)

returns the empty list instead of the two Trilead implementations, causing jenkinsci/acceptance-test-harness#1539 and jenkinsci/acceptance-test-harness#1540 (39 new ATH failures).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants