Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

some plugins could be optional #20

Merged

Conversation

jcoste-orange
Copy link

plain-credentials, google-oauth-plugin and docker-commons could be optional.

If I don't use GKE, I should not be forced to install google-oauth-plugin.

I tried to see how to add test but I couldn't find a way to test that the plugin behaves normally with or without these optionals plugins.

@jglick
Copy link
Member

jglick commented Mar 31, 2020

I couldn't find a way to test that the plugin behaves normally with or without these optionals plugins

I am afraid that is not really possible when using JenkinsRule. You need to do manual sanity checks. Be very careful. The way optional works in @Extension is that an error thrown while loading the extension is ignored (or at least logged at INFO). If the error is only thrown when using the extension, it is too late: you will have repeated errors at runtime. In this case I am guessing it will work since, e.g.,

super(KubernetesAuthToken.class, GoogleRobotCredentials.class);

must throw a NoClassDefFoundError before the extension instance can be instantiated. (The type parameter on the class declaration might cause this to be thrown earlier, when the class is resolved, but given that generics are not reified in Java I am not sure offhand.)

@jcoste-orange
Copy link
Author

Thanks Jesse for this quick response.

Here are my manuals testing :

Starts Jenkins with the plugin with mvn hpi:run (so all plugins are enabled).
Execute this groovy script +1

import jenkins.model.*
import hudson.model.*
import jenkins.authentication.tokens.api.AuthenticationTokenSource;

Jenkins.getInstance().getExtensionList(AuthenticationTokenSource.class).each{
  println it
}
return ""

The result is :

org.jenkinsci.plugins.docker.commons.impl.ServerKeyMaterialFactoryFromDockerCredentials@68703058
org.jenkinsci.plugins.docker.commons.impl.UsernamePasswordDockerRegistryTokenSource@16e51cc
org.jenkinsci.plugins.kubernetes.tokensource.DockerServerCredentialsTokenSource@7508ea4a
org.jenkinsci.plugins.kubernetes.tokensource.FileCredentialsTokenSource@c7ae937
org.jenkinsci.plugins.kubernetes.tokensource.GoogleRobotCredentialsTokenSource@65e1fa10
org.jenkinsci.plugins.kubernetes.tokensource.StandardCertificateCredentialsTokenSource@16f23751
org.jenkinsci.plugins.kubernetes.tokensource.StringCredentialsTokenSource@249ad57f
org.jenkinsci.plugins.kubernetes.tokensource.UsernamePasswordCredentialsTokenSource@7ff2b38a

Stops Jenkins. Then disable optional plugins.
touch work/plugins/docker-commons.jpi.disabled
touch work/plugins/google-oauth-plugin.jpi.disabled
touch work/plugins/oauth-credentials.jpi.disabled
touch work/plugins/plain-credentials.jpi.diabled
touch work/plugins/credentials-binding.jpi.disabled

Starts Jenkins wih mvn hpi:run.
One line of log output :

INFOS: Failed to instantiate optional component org.jenkinsci.plugins.kubernetes.credentials.FileSystemServiceAccountCredential$DescriptorImpl; skipping

Execute the same groovy script and got :

org.jenkinsci.plugins.kubernetes.tokensource.StandardCertificateCredentialsTokenSource@5e95811e
org.jenkinsci.plugins.kubernetes.tokensource.UsernamePasswordCredentialsTokenSource@5d185823

So, to me, it seems to work. On the other hand, there is not log about skipping GoogleRobotCredentialsTokenSource or DockerServerCredentialsTokenSource.

Do you think that's enough of a test ?

@jglick
Copy link
Member

jglick commented Mar 31, 2020

On the other hand, there is not log about skipping GoogleRobotCredentialsTokenSource or DockerServerCredentialsTokenSource.

Or StringCredentialsTokenSource or FileCredentialsTokenSource. This seems suspicious. I do not know of any reason why these four lines would not have been printed (at INFO). Best to try again in java -jar jenkins.war mode, since hpi:run mode (like JenkinsRule) uses a different class loading model than production: install Jenkins in a fresh $JENKINS_HOME, install kubernetes-credentials from the update center, from /pluginManager/advanced upload a *.hpi from this PR, restart then continue testing as you did.

@jcoste-orange
Copy link
Author

I did the same tests with a Jenkins 2.176.1 with fresh $JENKINS_HOME.
I've got the same results.

Just one line in the log for FileSystemServiceAccountCredential.
No log for any token source I've modified in this PR.

If I set FINE (PRÉCIS in french) log level, I then have some trace of LinkageError :

$ grep TokenSource jenking.log
PRÉCIS: Loaded index item org.jenkinsci.plugins.kubernetes.tokensource.DockerServerCredentialsTokenSource{optional=true}
PRÉCIS: Loaded index item org.jenkinsci.plugins.kubernetes.tokensource.FileCredentialsTokenSource{optional=true}
PRÉCIS: Loaded index item org.jenkinsci.plugins.kubernetes.tokensource.GoogleRobotCredentialsTokenSource{optional=true}
PRÉCIS: Loaded index item org.jenkinsci.plugins.kubernetes.tokensource.StandardCertificateCredentialsTokenSource
PRÉCIS: Loaded index item org.jenkinsci.plugins.kubernetes.tokensource.StringCredentialsTokenSource{optional=true}
PRÉCIS: Loaded index item org.jenkinsci.plugins.kubernetes.tokensource.UsernamePasswordCredentialsTokenSource
PRÉCIS: Failed to scout org.jenkinsci.plugins.kubernetes.tokensource.GoogleRobotCredentialsTokenSource
PRÉCIS: Loaded index item org.jenkinsci.plugins.kubernetes.tokensource.DockerServerCredentialsTokenSource{optional=true}
PRÉCIS: Loaded index item org.jenkinsci.plugins.kubernetes.tokensource.FileCredentialsTokenSource{optional=true}
PRÉCIS: Loaded index item org.jenkinsci.plugins.kubernetes.tokensource.GoogleRobotCredentialsTokenSource{optional=true}
PRÉCIS: Loaded index item org.jenkinsci.plugins.kubernetes.tokensource.StandardCertificateCredentialsTokenSource
PRÉCIS: Loaded index item org.jenkinsci.plugins.kubernetes.tokensource.StringCredentialsTokenSource{optional=true}
PRÉCIS: Loaded index item org.jenkinsci.plugins.kubernetes.tokensource.UsernamePasswordCredentialsTokenSource
PRÉCIS: Loaded index item org.jenkinsci.plugins.kubernetes.tokensource.DockerServerCredentialsTokenSource{optional=true}
PRÉCIS: Loaded index item org.jenkinsci.plugins.kubernetes.tokensource.FileCredentialsTokenSource{optional=true}
PRÉCIS: Loaded index item org.jenkinsci.plugins.kubernetes.tokensource.GoogleRobotCredentialsTokenSource{optional=true}
PRÉCIS: Loaded index item org.jenkinsci.plugins.kubernetes.tokensource.StandardCertificateCredentialsTokenSource
PRÉCIS: Loaded index item org.jenkinsci.plugins.kubernetes.tokensource.StringCredentialsTokenSource{optional=true}
PRÉCIS: Loaded index item org.jenkinsci.plugins.kubernetes.tokensource.UsernamePasswordCredentialsTokenSource
PRÉCIS: Failed to load org.jenkinsci.plugins.kubernetes.tokensource.DockerServerCredentialsTokenSource
java.lang.LinkageError: Failed to resolve class org.jenkinsci.plugins.kubernetes.tokensource.DockerServerCredentialsTokenSource
PRÉCIS: Failed to load org.jenkinsci.plugins.kubernetes.tokensource.FileCredentialsTokenSource
java.lang.LinkageError: Failed to resolve class org.jenkinsci.plugins.kubernetes.tokensource.FileCredentialsTokenSource
PRÉCIS: Failed to load org.jenkinsci.plugins.kubernetes.tokensource.GoogleRobotCredentialsTokenSource
java.lang.LinkageError: Failed to resolve class org.jenkinsci.plugins.kubernetes.tokensource.GoogleRobotCredentialsTokenSource
PRÉCIS: Failed to load org.jenkinsci.plugins.kubernetes.tokensource.StringCredentialsTokenSource
java.lang.LinkageError: Failed to resolve class org.jenkinsci.plugins.kubernetes.tokensource.StringCredentialsTokenSource

Do you think it's OK ?

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.

OK. Looks like there is some inconsistency in how ExtensionFinder sets log levels here. No idea why GoogleRobotCredentials is treated differently, but Guice is basically a black box.

@maxlaverse
Copy link

Thanks to both of you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants