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

Seed job "external" credential type always requires k8s secret to be defined #552

Closed
adaphi opened this issue May 3, 2021 · 6 comments
Closed
Assignees
Labels
bug Something isn't working
Milestone

Comments

@adaphi
Copy link

adaphi commented May 3, 2021

Describe the bug
Hey,

I think this counts as a bug, but if not then feel free to reclassify.

When using the "external" credentialType for seed job config, the operator throws an error if there is no Kubernetes Secret defined with the name given in the credentialID. I'm not sure if this is intentional, but I assume it's not, since when using the "external" type, the Secret is completely ignored. From the (minimal) documentation it appears to expect the credential will be created from another source (such as CasC configs). Should it be requiring the existence of a k8s Secret on top of that?

The error:

seedJob `my-seed-job` required secret 'git-ssh-vault' with Jenkins credential not found

In our case, we are using CasC to define credentials that are pulled from Hashicorp Vault. By using the external type, we can let the seed jobs use those credentials, but in order to do so we have to create a completely empty secret with the same name as the credential ID just to get past the error. Once that's done, the seed job runs as expected, so we have a good workaround.

The check is here:
https://github.com/jenkinsci/kubernetes-operator/blob/master/pkg/configuration/user/seedjobs/validate.go#L59

To Reproduce
Create a seed job config with the external type and a credentialID that is created from another source (such as CasC)

  seedJobs:
  - id: jenkins-operator-ssh
    credentialType: external
    credentialID: git-ssh-vault
    targets: "cicd/jobs/*.jenkins"
    description: "Jenkins Operator repository"
    repositoryBranch: master
    repositoryUrl: ssh://git@github.com:jenkinsci/kubernetes-operator.git

Observe that the operator throws the error

seedJob `jenkins-operator-ssh` required secret 'git-ssh-vault' with Jenkins credential not found

Create that secret

apiVersion: v1
kind: Secret
metadata:
  annotations:
    comment: "This secret intentionally left blank"
  name: git-ssh-vault

Observe that the error is no longer thrown

Additional information

Kubernetes version: 1.18.10
Jenkins Operator version: helm chart 0.4.3, app version 0.5.0

Couple of other things worth mentioning:

The documentation for this feature is very light, with no examples. It would be nice it some detail could be added - at the moment I'm just guessing at what the intended functionality is supposed to be, so its possible I'm assuming something wrong about the intended use-case here.

Also I notice that the code does not check that you've set a credential ID when using the external type. Is that intended? I'm not sure what it would do in such a case - perhaps the clause seedJob.JenkinsCredentialType == v1alpha2.ExternalCredentialType is just in the wrong if statement?

@adaphi adaphi added the bug Something isn't working label May 3, 2021
@leifmadsen
Copy link

I'm having a similar issue to this, and I'm not sure if they are related or not. If you pass your own credentials (like for say a GitHub App), then it overwrites any generated credentials, like the one used for authentication to the system to execute jobs on the underlying Kubernetes infrastructure. So even if you get the seed job to run, the jobs themselves will fail because it's unable to create a pod to run the jobs.

(Apologies if this is unrelated, I'm not trying to hijack this issue, but maybe this configuration is of some use?)

In my case I create a ConfigMap that refers to a Secret with the containing RSA key for authentication to GitHub. The ConfigMap looks like this:

apiVersion: v1
data:
  10-gh-app-credentials.yaml: |
    credentials:
      system:
        domainCredentials:
          - credentials:
            - gitHubApp:
                appID: "12345678"
                description: "GitHub app"
                id: "github-app"
                privateKey: "${GITHUB_APP_KEY}"
                scope: SYSTEM
kind: ConfigMap
...

And the Secret:

apiVersion: v1
data:
  GITHUB_APP_KEY: LS0tLS1CR...S0tLS0K
kind: Secret
...

And the configuration in the Operator to load this:

...
Kind: Jenkins
...
spec:
  configurationAsCode:
    configurations:
      - name: jenkins-operator-user-configuration
    secret:
      name: github-app-key

@prryb
Copy link
Collaborator

prryb commented May 4, 2021

Hey @adaphi, this definitely looks like a bug. That check shouldn't be there. There are some examples in the docs for the kubernetes-credentials-provider-plugin (https://jenkinsci.github.io/kubernetes-credentials-provider-plugin/examples/) that the operator is using. You won't find external there though - that's something the operator is adding. +1 for extending documentation on this topic.

Hi @leifmadsen, could you open a new issue? I'll try to look into it :)

@prryb prryb self-assigned this May 4, 2021
@prryb
Copy link
Collaborator

prryb commented May 4, 2021

Ok, I've found that existing docs under Getting Started/Latest/Configuration describe supported credential types in more detail, and with examples (https://jenkinsci.github.io/kubernetes-operator/docs/getting-started/latest/configuration/). Would you consider it sufficient? I don't think I could add much more to that.

@adaphi
Copy link
Author

adaphi commented May 4, 2021

Hi @prryb, thanks for the response and the quick action!

Ok, I've found that existing docs under Getting Started/Latest/Configuration describe supported credential types in more detail, and with examples (https://jenkinsci.github.io/kubernetes-operator/docs/getting-started/latest/configuration/). Would you consider it sufficient? I don't think I could add much more to that.

I think it is sufficient for the SSH and user/pass sections. Personally I think the external section needs a little more? I found I had to go and read the code in order to understand how to use it :) IMO it just needs an example of the seedJobs config with external as the type, showing the parameters that are needed (including credential ID), maybe with a note saying that the credential ID must be match one that is created in Jenkins from another source? That's just my feeling though, if you think it is clear enough then I won't press for it :)

@prryb
Copy link
Collaborator

prryb commented May 4, 2021

Cool, docs updated in the PR :)

prryb added a commit that referenced this issue May 5, 2021
* Don't validate external credential type

The operator shouldn't try to fetch credentials that have their types
defined as `external` - that means that credentials are supplied
externally, without using k8s secrets.

* Docs: Add example of `external` credential type
@KorusMateusz
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants