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

Secrets username, password (like image source secrets) require! Annotations. Else: NPE #293

Closed
jomeier opened this issue Mar 15, 2019 · 12 comments · Fixed by #304
Closed

Comments

@jomeier
Copy link

jomeier commented Mar 15, 2019

Hi,

jenkins-sync-plugin 1.0.34 require this annotation to be set:

  annotations:
    jenkins.openshift.io/secret.name: <something>

Without that I get this NPE:

Mar 15, 2019 9:57:22 AM org.jenkinsci.plugins.workflow.job.WorkflowRun finish
  | INFO: cocdit-cicd-qual/cocdit-cicd-qual-pipeline-build #5 completed: FAILURE
  | Mar 15, 2019 9:57:22 AM io.fabric8.jenkins.openshiftsync.BuildSyncRunListener onCompleted
  | INFO: onCompleted job/cocdit-cicd-qual/job/cocdit-cicd-qual-pipeline-build/5/
  | Mar 15, 2019 9:57:22 AM io.fabric8.jenkins.openshiftsync.BuildWatcher eventReceived
  | WARNING: Caught: java.lang.NullPointerException
  | java.lang.NullPointerException
  | at io.fabric8.jenkins.openshiftsync.JenkinsUtils.deleteRun(JenkinsUtils.java:566)
  | at io.fabric8.jenkins.openshiftsync.JenkinsUtils.deleteRun(JenkinsUtils.java:575)
  | at io.fabric8.jenkins.openshiftsync.BuildWatcher.innerDeleteEventToJenkinsJobRun(BuildWatcher.java:424)
  | at io.fabric8.jenkins.openshiftsync.BuildWatcher.deleteEventToJenkinsJobRun(BuildWatcher.java:453)
  | at io.fabric8.jenkins.openshiftsync.BuildWatcher.eventReceived(BuildWatcher.java:171)
  | at io.fabric8.jenkins.openshiftsync.BuildWatcher.eventReceived(BuildWatcher.java:187)
  | at io.fabric8.jenkins.openshiftsync.WatcherCallback.eventReceived(WatcherCallback.java:34)
  | at io.fabric8.kubernetes.client.dsl.internal.WatchConnectionManager$2.onMessage(WatchConnectionManager.java:237)
  | at okhttp3.internal.ws.RealWebSocket.onReadMessage(RealWebSocket.java:310)
  | at okhttp3.internal.ws.WebSocketReader.readMessageFrame(WebSocketReader.java:222)
  | at okhttp3.internal.ws.WebSocketReader.processNextFrame(WebSocketReader.java:101)
  | at okhttp3.internal.ws.RealWebSocket.loopReader(RealWebSocket.java:265)
  | at okhttp3.internal.ws.RealWebSocket$2.onResponse(RealWebSocket.java:204)
  | at okhttp3.RealCall$AsyncCall.execute(RealCall.java:153)
  | at okhttp3.internal.NamedRunnable.run(NamedRunnable.java:32)
  | at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
  | at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
  | at java.lang.Thread.run(Thread.java:748)
  |  
  | Mar 15, 2019 9:57:22 AM io.fabric8.jenkins.openshiftsync.BuildSyncRunListener onFinalized
  | INFO: onFinalized job/cocdit-cicd-qual/job/cocdit-cicd-qual-pipeline-build/5/
  | Mar 15, 2019 9:57:24 AM io.fabric8.jenkins.openshiftsync.BuildConfigWatcher eventReceived
  | WARNING: Caught: java.lang.NullPointerException
  | java.lang.NullPointerException
  | at io.fabric8.jenkins.openshiftsync.CredentialsUtils.updateSourceCredentials(CredentialsUtils.java:62)
  | at io.fabric8.jenkins.openshiftsync.BuildConfigToJobMapper.mapBuildConfigToFlow(BuildConfigToJobMapper.java:92)
  | at io.fabric8.jenkins.openshiftsync.BuildConfigWatcher$3.call(BuildConfigWatcher.java:263)
  | at io.fabric8.jenkins.openshiftsync.BuildConfigWatcher$3.call(BuildConfigWatcher.java:238)
  | at hudson.security.ACL.impersonate(ACL.java:290)
  | at io.fabric8.jenkins.openshiftsync.BuildConfigWatcher.upsertJob(BuildConfigWatcher.java:238)
  | at io.fabric8.jenkins.openshiftsync.BuildConfigWatcher.modifyEventToJenkinsJob(BuildConfigWatcher.java:351)
  | at io.fabric8.jenkins.openshiftsync.BuildConfigWatcher.eventReceived(BuildConfigWatcher.java:173)
  | at io.fabric8.jenkins.openshiftsync.BuildConfigWatcher.eventReceived(BuildConfigWatcher.java:231)
  | at io.fabric8.jenkins.openshiftsync.WatcherCallback.eventReceived(WatcherCallback.java:34)
  | at io.fabric8.kubernetes.client.dsl.internal.WatchConnectionManager$2.onMessage(WatchConnectionManager.java:237)
  | at okhttp3.internal.ws.RealWebSocket.onReadMessage(RealWebSocket.java:310)
  | at okhttp3.internal.ws.WebSocketReader.readMessageFrame(WebSocketReader.java:222)
  | at okhttp3.internal.ws.WebSocketReader.processNextFrame(WebSocketReader.java:101)
  | at okhttp3.internal.ws.RealWebSocket.loopReader(RealWebSocket.java:265)
  | at okhttp3.internal.ws.RealWebSocket$2.onResponse(RealWebSocket.java:204)
  | at okhttp3.RealCall$AsyncCall.execute(RealCall.java:153)
  | at okhttp3.internal.NamedRunnable.run(NamedRunnable.java:32)
  | at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
  | at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
  | at java.lang.Thread.run(Thread.java:748)
 
@waveywaves
Copy link

@jomeier Thank you for opening up this issue. Has this issue surfaced only after updating the jenkins sync plugin ?

cc @gabemontero
wrt #283

@jomeier
Copy link
Author

jomeier commented Mar 15, 2019

@waveywaves
No. I compiled the openshift-jenkins Docker Image with the newest version of the Plugin.

@gabemontero
Copy link

Hmm ... the second NPE's line number

  | java.lang.NullPointerException
  | at io.fabric8.jenkins.openshiftsync.CredentialsUtils.updateSourceCredentials(CredentialsUtils.java:62)

does not line up with the latest version of the code, which would be v1.0.34. That line,

Now, the prior version of that file, before my fix, which would have been v1.0.33, does point to the incorrect assumption that the annotation would be non-null. See

sourceSecret.getMetadata().getAnnotations().get(Annotations.SECRET_NAME));

Now, java stack traces certainly have not been full proof over the years. But it being off by one, say up one, would imply the metadata was null ... that should definitely not be the case. See

Down one, at

BuildConfigSecretToCredentialsMap.linkBCSecretToCredential(NamespaceName.create(buildConfig).toString(),

An NPE can't occur from that NamespaceName.create(..) call. It instantiates an obj, or thows and illegal arg exception.

@jomeier .... are you SURE you were at v1.0.34 when you saw the second NPE? What was the SHA of the openshift/jenkins docker image you pulled ?

Certainly the intent of v1.0.34 was to correct the assumption that the annotation map would be there.

The first NPE, in isolation, is unrelated to secret annotations altogether. However, maybe the failure from the credential NPEs corrupted sync plugin data structures and caused the first one. They occurred simultaenously, even if they showed up in the log in the order they did. Also, the second was on a BC event, so I could see that correlation.

Let's wait on @jomeier 's response to the image sha / confirmation on the plugin version, and we'll go from there.

@jomeier
Copy link
Author

jomeier commented Mar 19, 2019

@gabemontero
Hi Gabe, I made further tests and found out that maby there was a problem with my custom build of the OpenShift Jenkins Image. Somehow an older version of the OpenShift Sync Plugin was copied into the Plugin directory of Jenkins and mixed up with version 1.0.34. We removed that.

Now it seems to work fine. Thank you for assisting me!

Greetz,

Josef

@jomeier jomeier closed this as completed Mar 19, 2019
@gabemontero
Copy link

ok good news @jomeier thanks for the update

@jomeier
Copy link
Author

jomeier commented Mar 19, 2019

@gabemontero
Sorry for bothering you again but...


Mar 19, 2019 3:00:44 AM io.fabric8.jenkins.openshiftsync.CredentialsUtils upsertCredential
--
  | INFO: Updated credential cocdit-cicd-qual-gitlab-pull-secret from Secret NamespaceName{cocdit-cicd-qual:gitlab-pull-secret} with revision: 24512366
  | Mar 19, 2019 3:00:44 AM io.fabric8.jenkins.openshiftsync.SecretWatcher onInitialSecrets
  | SEVERE: Failed to update job
  | java.lang.NullPointerException
  | at java.lang.String.<init>(String.java:515)
  | at io.fabric8.jenkins.openshiftsync.CredentialsUtils.newSecretTextCredential(CredentialsUtils.java:327)
  | at io.fabric8.jenkins.openshiftsync.CredentialsUtils.arbitraryKeyValueTextCredential(CredentialsUtils.java:234)
  | at io.fabric8.jenkins.openshiftsync.CredentialsUtils.secretToCredentials(CredentialsUtils.java:289)
  | at io.fabric8.jenkins.openshiftsync.CredentialsUtils.upsertCredential(CredentialsUtils.java:120)
  | at io.fabric8.jenkins.openshiftsync.CredentialsUtils.upsertCredential(CredentialsUtils.java:110)
  | at io.fabric8.jenkins.openshiftsync.SecretWatcher.upsertCredential(SecretWatcher.java:165)
  | at io.fabric8.jenkins.openshiftsync.SecretWatcher.onInitialSecrets(SecretWatcher.java:122)
  | at io.fabric8.jenkins.openshiftsync.SecretWatcher.access$100(SecretWatcher.java:37)
  | at io.fabric8.jenkins.openshiftsync.SecretWatcher$1.doRun(SecretWatcher.java:69)
  | at hudson.triggers.SafeTimerTask.run(SafeTimerTask.java:72)
  | at jenkins.security.ImpersonatingScheduledExecutorService$1.run(ImpersonatingScheduledExecutorService.java:58)
  | at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
  | at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
  | at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180)
  | at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294)
  | at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
  | at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
  | at java.lang.Thread.run(Thread.java:748)


With the annotation which changes the name I dont get this.

There still seems to be a problem....

@jomeier jomeier reopened this Mar 19, 2019
@gabemontero
Copy link

An NPE at that line actually speaks to the secret text, not the secret name that is driven by the annotation. I suspect that there is another variable in play, but can't say for sure, in addition to what you observed.

See

hudson.util.Secret.fromString(new String(Base64.decode(secretText), StandardCharsets.UTF_8)));

An NPE in the String contstructor means secretText is null, or the base64 encoding of it is null

Either does not make much sense given the null checks preceding the line, and what I see in the Base64.decode logic.

secretText is actually populated in the arbitraryKeyValueTextCredential given the type of secret you supplied

Could you possibly provide yaml for

  1. the openshift secret without the annotation
  2. the openshift secret with the annotation (I want to double check how you are setting the annotation just in case)

If you need to obfuscate any sensitive data, or course please do.

Otherwise, other than putting a try catch around that line for debug, not sure what else I'm would do here.

thanks

@nilsding
Copy link

nilsding commented Apr 9, 2019

Hello everyone,

I just ran into the same (or a similar, but maybe related) issue and looked into the CredentialsUtils.java a bit more closely.

From my understanding, the method newSecretTextCredential(String secretName, String secretText) expects secretText to be a base64 encoded string. However the method arbitraryKeyValueTextCredential(Map<String, String> data, String secretName) does not seem to base64 encode the resulting JSON object before calling that method as of now :

private static Credentials arbitraryKeyValueTextCredential(Map<String, String> data, String secretName) {
String text = "";
if (data != null && data.size() > 0) {
// convert to JSON for parsing ease in pipelines
try {
text = new ObjectMapper().writeValueAsString(data);
} catch (JsonProcessingException e) {
logger.log(Level.WARNING, "Arbitrary opaque secret " + secretName + " had issue converting json", e);
}
}
if (StringUtils.isBlank(text)) {
logger.log(
Level.WARNING,
"Opaque secret {0} did not provide any data that could be processed into a Jenkins credential",
new Object[] { secretName });
return null;
}
return newSecretTextCredential(secretName, text);
}

The secret I used to reproduce the backtrace from @jomeier looks like this:

apiVersion: v1
data:
  SECRET_A: bm9wZS4=
  SECRET_B: bm9wZS4=
  SECRET_C: bm9wZS4=
  SECRET_D: bm9wZS4=
kind: Secret
metadata:
  labels:
    credential.sync.jenkins.openshift.io: "true"
  name: omgsosecret
  namespace: my-namespace
type: Opaque

and here's the backtrace I got for completeness:

Apr 09, 2019 9:33:40 AM io.fabric8.jenkins.openshiftsync.SecretWatcher onInitialSecrets
SEVERE: Failed to update job
java.lang.NullPointerException
	at java.lang.String.<init>(String.java:515)
	at io.fabric8.jenkins.openshiftsync.CredentialsUtils.newSecretTextCredential(CredentialsUtils.java:327)
	at io.fabric8.jenkins.openshiftsync.CredentialsUtils.arbitraryKeyValueTextCredential(CredentialsUtils.java:234)
	at io.fabric8.jenkins.openshiftsync.CredentialsUtils.secretToCredentials(CredentialsUtils.java:278)
	at io.fabric8.jenkins.openshiftsync.CredentialsUtils.upsertCredential(CredentialsUtils.java:120)
	at io.fabric8.jenkins.openshiftsync.CredentialsUtils.upsertCredential(CredentialsUtils.java:110)
	at io.fabric8.jenkins.openshiftsync.SecretWatcher.upsertCredential(SecretWatcher.java:165)
	at io.fabric8.jenkins.openshiftsync.SecretWatcher.onInitialSecrets(SecretWatcher.java:122)
	at io.fabric8.jenkins.openshiftsync.SecretWatcher.access$100(SecretWatcher.java:37)
	at io.fabric8.jenkins.openshiftsync.SecretWatcher$1.doRun(SecretWatcher.java:69)
	at hudson.triggers.SafeTimerTask.run(SafeTimerTask.java:72)
	at jenkins.security.ImpersonatingScheduledExecutorService$1.run(ImpersonatingScheduledExecutorService.java:58)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)

Hope this helps a bit ^^

@gabemontero
Copy link

@waveywaves - given you being the genesis of the new annotation, how about you see if you can leverage @nilsding 's reproducer secret to generate the same NPE with the latest version of sync plugin.

based on the results, we can go from there and see if you can submit a fix.

as a reminder, the end to end process is at https://github.com/openshift/jenkins/blob/master/CONTRIBUTING_TO_OPENSHIFT_JENKINS_IMAGE_AND_PLUGINS.md

@pradeepto @sthaha @pweil- fyi

@waveywaves
Copy link

waveywaves commented Apr 11, 2019

@nilsding I was able to reproduce this in v1.0.34 where the annotation feature for changing the secret name has been implemented as well as in v1.0.32 where it wasn't. To ensure that you have a working secret in Jenkins you have to create the openshift secret in the following way.

1> Create a file with you secrets.

$ cat secret
SECRET1=nope.
SECRET2=nope.
SECRET3=nope.
SECRET4=nope.

2> Create an Openshift Secret using the following command

$ oc create secret generic omgsosecret --from-file=filename=secret

3> Label the secret

$ oc label secret omgsosecret credential.sync.jenkins.openshift.io=true

Your secret should be usable in your jenkins instance.

@gabemontero This doesn't seem to be an issue with the new annotation, but with how the secrets are parsed as @nilsding mentioned.
As of now we can confirm that this is not happening because of the annotation itself, but because of how the secrets are beeing parsed.

Will look into it further.

@gabemontero
Copy link

bump @waveywaves .... how far along are you with addressing the encoding issue (since the annotation has been proven to be a unrelated thing along the same code path) ?

@gabemontero
Copy link

v1.0.36 of the sync plugin has bee initiated at the jenkins update center

it contains the fix from @waveywaves via #304

@nilsding @jomeier if either of you can try v1.0.36 in the near term and let us know what you see that will be great

we are still weighing when to update the openshift/jenkins images given our proximity to 4.0/4.1 GA

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 a pull request may close this issue.

4 participants