Allow using classes not supported by structs for DescribableContext

[twz123]

This allows for types that are not directly supported by structs to be used in Job DSL. An example for this is hudson.util.Secret, that's used by by the ssh-credentials plugin. See JENKINS-57435 for details.

[darxriggs]

@twz123 Can you also implement a test for this change?

[twz123]

@darxriggs done

[darxriggs]

@daspilker can you have a look at this?

[jsknnr]

This would be super neat.

[jwyoungpm]

Bump on this as well, would be very helpful.

[hypePG]

bump! any plan to merge this to master?

[jetersen]

Yup, this is a blocker for hashicorp vault plugin 😓

[daspilker]

Hm, I not sure if it's a good idea to open the floodgates so that hudson.util.Secret can be used as type in Dynamic DSL. That will lead to passwords being exposed in scripts.

Is there no other solution possible? Should Job DSL really be used for managing credentials? It's probably not good at that and has not been designed for managing credentials. There may be other security implications.

[twz123]

We are using Job DSL via CasC to provision secrets into folders. The secrets themselves are not part of the scripts, but, when run on the master node, the scripts are able to access them there. I reckon it's a pretty common approach to do such things when using CasC. Please correct me if I'm wrong.

It was already possible to use Job DSL for SSH secrets until April 2019, when SSH Credentials Plugin 1.16 was released, and is actually still usable now, when falling back to XML config, as shown here. This change is not specifically about secrets, but generally about any type that is not supported by structs. The secret use case was just my personal driver for the PR.

[daspilker]

Yes, it is possible to put secrets in DSL scripts, but IMHO Job DSL does not need a more convenient ways to do that.

When done properly Job DSL would need support for all kinds of vaults. Given hat CasC already has support for vaults, wouldn't it make sense to use CasC directly for provisioning of folders? E.g. by adding a CasC extension to the Folders plugin?

[twz123]

Problem for us is that folders are determined dynamically via calls to external APIs. When not using Job DSL for this, we'd need to introduce some custom ad-hoc third layer before CasC and Job DSL that actually preprocesses CasC config files before applying them.

[daspilker]

It's 2020, generating YAML files is an industry-wide best practice. CasC should have an answer to that... 😏

@twz123 I see your point, how do you get the secrets into the script?

[jetersen]

JCasC should not be generating yaml. It should stay declarative. 😉

There are so many tools out there that can generate yaml that is true.

[jetersen]

@daspilker secrets are provided by JCasC secret source resolver:

job-dsl-plugin/job-dsl-plugin/src/main/groovy/javaposse/jobdsl/plugin/casc/SeedJobConfigurator.java

Line 112 in b3bd348

return SecretSourceResolver.resolve(context, value);

https://github.com/jenkinsci/configuration-as-code-plugin/blob/master/docs/features/secrets.adoc

[twz123]

We're mounting Kubernetes secrets into the Jenkins master, as described here.

[daspilker]

If secrets are available as environment variable, Dynamic DSL could convert a variable name to a hudson.util.Secret by taking the variable value and invoking Secret.fromString. That would mitigate the risk of leaking secrets into scripts because the secret itself must not be processed in the script. Would that be feasible from a user's point of view?

folder('foo') {
  properties {
    folderCredentialsProperty {
      domainCredentials {
        domainCredentials {
          // ...
          credentials {
            basicSSHUserPrivateKey {
              // ...
              privateKeySource {
                directEntryPrivateKeySource {
                  privateKey('MY_SECRET_ENV_VAR')
                }
              }
            }
          }
        }
      }
    }
  }
}

[jetersen]

The thing is JCasC does not offer secrets as environment variables. I am pretty sure that is undesired.
Of course users of JCasC is free to add their secrets as environment variables.

[jetersen]

@daspilker I would prefer that this was supported and if the user decides to write secrets in plain text that's their own risk.

[jsknnr]

The problem I am facing is that I am scoping credentials to folders that I am also building with job-dsl. So If the DSL does not include the credential as part of the plan for the folder, then I cannot simply manually put them in there as next time my job to apply updates to the DSL runs it will overwrite the manually added credentials.

I first faced this when trying to integrate Vault for secret management. I worked around that by instead of using the actual plugin, calling the API directly and passing it a user and password. I am able to currently do this with job-dsl since the Credentials plugin UsernamePasswordCredentialsImpl expects the password to actually be a string instead of the hudson.util.Secret type. I understand that the latter may be preferred.

I do not want to manage secrets with Jenkins per se, but due to the work that Jenkins does, and how I choose to manage Jenkins, it does occasionally need a secret. And no, I am not committing plain text secrets to source control - they are encrypted with KMS before commit and decrypted at run time so Jenkins can access it.

[fugkco]

Somewhat related jenkinsci/plain-credentials-plugin#23 (note I'm not sure if my PR is the right solution)

I totally understand the point of prevent people from committing secrets into git or whatnot, however, I think this is going to be somewhat solved by letting people know, plus some examples of how to do it properly.

In our case we're using job-dsl to manage folders and jobs, and the plan is to have a helper class as part of additionalClasspath that retrieves secrets from a secrets source (for us it's AWS SSM Parameter Store) and then pass it into the job-dsl folder block, something like this:

folder('my-tester') {
    properties {
        folderCredentialsProperty {
            domainCredentials {
                domainCredentials {
                    domain {
                        name("test")
                        description("Credentials necessary for our tests")
                    }
                    credentials {
                        stringCredentialsImpl {
                            scope("GLOBAL")
                            id("id")
                            description("")
                            secret(Ssm.getSecret('/path/to/mysecret'))
                        }
                    }
                }
            }
        }
    }
}

Re managing folders in JCasC: in my opinion, having to manage folders through JCasC, and jobs through job-dsl will make things unnecessarily complicated.

If secrets are available as environment variable, Dynamic DSL could convert a variable name to a hudson.util.Secret by taking the variable value and invoking Secret.fromString. That would mitigate the risk of leaking secrets into scripts because the secret itself must not be processed in the script. Would that be feasible from a user's point of view?

While that's fine, it doesn't capture all use cases, there are other places that can store secrets, such as AWS SSM Parameter Store as in our case, but also AWS Secrets Manager, Vault, and potentially others. In addition relying on environment variables would require Jenkins to restart to see any new secrets, compared to simply doing a query to SSM or other secrets store, which allows a credential to change by simply running a jobDsl step without restarting Jenkins.

Note, personally I prefer this approach instead of my aforementioned PR.

[RiccardoBoettcher]

Another solution: JobDSL can be used to create folders. But if the folder to be created already exists, it doesn't modify it. Actually JobDSL removes manually configured secrets of any kind which is very, very anoying.
Use case: boot-strap a Jenkins config via JCasC + JobDSL for creation of initial seed jobs. In this scenario the JobDSL is executed at every start of Jenkins. Thus manually added secrets vanish with each restart of Jenkins.

[fugkco]

@jglick @oleg-nenashev sorry to ping directly. It seems @daspilker might have disappeared. Any chance this PR can be reviewed and merged if all looks okay?

[jglick]

I am not a maintainer.

[twz123]

Coming back to this again, gently pinging @daspilker. The discussion focused a lot on the secret use case, although this PR is really more general in nature, as it's adding support for any type which wouldn't be usable otherwise from within job-dsl.

I share the concerns about leaking credentials. This can happen far too quickly, but I don't think that this change to job-dsl will make it worse. Do you have any suggestions in what we could improve here? Maybe there's some docs that could be written?

[Penson122]

@jamietanna this change would be extremely helpful for me and I'm sure others.
Automation of folder scoped credentials, particularly for secret text credentials via PlainCredentials is not really possible without some workaround.

I've been seeing some recent activity in this repo and just want to bring this to your attention and see where you stand on getting this merged?

[jamietanna]

Hey folks!

Just picking up from @RiccardoBoettcher's comment:

Another solution: JobDSL can be used to create folders. But if the folder to be created already exists, it doesn't modify it. Actually JobDSL removes manually configured secrets of any kind which is very, very anoying.

As per #1232 this is now resolved - does this change the viewpoint of folks significantly or is this still something that'd be useful to have?

(Please react to this message, or reply with justifications 👍)

I'll re-review this and see if there's anything else we can do education wise, or if there's anything we can do to make things more secure to apply Daniel's point of view.

Although I've stepped up to maintain while Daniel isn't available, I don't want to necessarily make decisions that wouldn't be appropriate according to them

[Penson122]

Thanks for getting back so quickly, but #1232 is helpful in the fact that folder scoped credentials are no longer removed when the folder closures are called.

However when using JCasC to create and manage credentials, it's also helpful to manage folder scoped ones too, as it stands (to my knowledge at least) there is no way to create new folder scoped credentials or keep them synced to some state in pure JCasC. Instead I have been using job top level type to manage credentials, see this example in the jcasc issues.

[fugkco]

I don't think that solves the issue as it requires creating credentials manually. It won't allow for configuring jenkins fully from a git repo. Allowing creation of credentials via this PR allows for pulling in secrets from any source (subject to custom coding), and allows to configure jenkins from an empty jenkins home if needed. In my opinion keeping a folder config is worse behaviour than what Job DSL plugin allows. Everywhere else it resets the config, but now only folders keep the config? Sounds counter intuitive.

[dduportal]

Hello there ! In order for the Jenkins infrastructure team to maintain all the Jenkins instances, this feature would help us a lot.

Ref. jenkins-infra/helpdesk#2840.

Currently, we are working on generating ourselves the job-dsl template from YML and it is really hard to rely on the "XML" annotation.

Of course, a pure JCasc solution would help, and of coure there is a risk of exposing credentials but as stated above, since the credentials are usually reachable by the system user that runs Jenkins (JCasc context in VMs or Kubernetes), the threat model is not impacted by this change on Job-DSL.
(I don't know for non JCasc usages though).

Thanks in advance for your work! If there is anything we can do to help / beta-test this, please reach out to us here on in IRC #jenkins-infra

[jamietanna]

Hey folks, apologies for the delay on this. I'll try and get a read through this in the next week or so, and if it's merged I'll immediately release 👍

[jamietanna]

@fugkco via your message here

It's odd to have folders merge but every other thing is completely replaced. That's very counter intuitive

I don't quite agree. Job DSL should be keeping the Jenkins configuration consistent, and I would say aside from manually updated secrets, nothing else should be persisted that had been manually configured.

---

I do agree with the risk that this will lead to potential leakage of secrets, and that we want to avoid this. However, I do appreciate that there's a lot of folks asking for this feature, and so I'll make sure that this is well-documented as a potentially risky thing to do.

Unless I hear anything strongly against this, I'll look to get this merged + released tomorrow.

[fugkco]

@jamietanna how will this work with the folder changes in that case? If I specify a credential through this change, and I run the seed job, it would of course add the credential to the folder. If I then re-ran the seed job, wouldn't that now have a credential on the folder, and then attempt to add the same credential on the folder from the DSL, thus causing errors?

[jamietanna]

If you're able to repro that and see what happens that'd be appreciated, I've not got anything handy to validate with

[fugkco]

I just had a test and it seems to be behaving as expected. I've not exactly tested the "keeping of old credentials", but as far as defining credentials in the folder, it seems to be removing creds as you remove them from the DSL, and added them once, and once only. No duplication or errors. Thanks @jamietanna and @twz123!