jenkinsci / configuration-as-code-plugin

Jenkins Configuration as Code Plugin
https://plugins.jenkins.io/configuration-as-code
MIT License
2.7k stars 722 forks source link

A wrong configuration of CasC yaml kills Jenkins startup #912

Open MRamonLeon opened 5 years ago

MRamonLeon commented 5 years ago

Your checklist for this issue

🚨 Please review the guidelines for contributing to this repository.

Description

While testing the CasC plugin I've found that if the yaml is wrong, Jenkins stop restarting, leaving you with an unoperational instance.

Steps to reproduce:

Cre

Expected: Some settings are established, others not.

Current behavior: Jenkins doesn't start.

Also filed in Jira: https://issues.jenkins-ci.org/browse/JENKINS-57862

jetersen commented 5 years ago

This is intentional behavior that was initially designed. Though would be great if we could improve it. :sweat: We should send the errors to an AdministrativeMonitor and continue consuming the next yaml bits. Depending on how critical we could instead choose to put Jenkins into the quiet mode to make it very visible that you have an improperly configured instance.

rudolfwalter commented 5 years ago

@MRamonLeon I have personally worked around this by applying CasC configs through a job, and disabling CasC for Jenkins startup by pointing it to an empty file.

That is, I have a job that does a SCM checkout, and then runs something like this as a system-groovy script (from the groovy plugin):

import jenkins.model.GlobalConfiguration
import io.jenkins.plugins.casc.ConfigurationAsCode
import io.jenkins.plugins.casc.CasCGlobalConfig
def workspacePath = build.environment.WORKSPACE
def jcascConfig = GlobalConfiguration.all().get(CasCGlobalConfig.class)
try {
    jcascConfig.setConfigurationPath(workspacePath)
    jcascConfig.save()
    ConfigurationAsCode.get().configure()
} finally {
    jcascConfig.setConfigurationPath("${workspacePath}/empty.yaml")
    jcascConfig.save()
}

(Note that below version 1.20, you need to have at least one setting in empty.yaml, a completely empty file generates a null pointer exception.)

MRamonLeon commented 5 years ago

This is intentional behavior that was initially designed. Though would be great if we could improve it. We should send the errors to an AdministrativeMonitor and continue consuming the next yaml bits. Depending on how critical we could instead choose to put Jenkins into the quiet mode to make it very visible that you have an improperly configured instance.

I also thought about AdministrativeMonitor before seeing your answer. So I'm more convinced it should be the path to follow, also with the logs and even the quiet mode for extreme cases. I know all the things have their history behind, but leaving the instance down seems excessive to me.

jansohn commented 5 years ago

@rudolfwalter how do you do the SCM checkout without any credentials initialized?

rudolfwalter commented 5 years ago

@jansohn In my case specifically, the setup is only 99% automated: while 100% of the configuration is applied through CasC, there is a small manual bootstrapping phase when creating a new Jenkins instance. It involves manually creating one credential (for the CasC SCM) and one job (that checks out and applies CasC). Both are defined within the CasC yaml files as well, so they get overwritten from SCM.

Note that during the initial setup, I don't manually go through the Jenkins setup GUI, because (a) I have scripts that automatically pre-copy all the plugin jpi files I need into the master's plugins folder and (b) I run Jenkins with -Djenkins.install.runSetupWizard=false.

Smasherr commented 5 years ago

I am with the AdministrativeMonitor-team.

timja commented 5 years ago

Sounds reasonable, happy to look at this on a pull request.

uhafner commented 5 years ago

I think it makes sense to look at Jenkins core on how to handle errors in configuration files, maybe you can adapt that part of the code. If a config.xml is broken then we get nice error messages in the UI rather than a bricked instance.

amuniz commented 3 years ago

Moving forward with start up sounds good to me, however there are some specific configuration sections that should be required to pass, ie. security - letting an instance start when security related configuration was not correctly applied is a security issue.

jglick commented 3 years ago

letting an instance start when security related configuration was not correctly applied is a security issue

FYI https://github.com/jenkinsci/jenkins/blob/f48c5f552f72485658c1c98482b42ae42ed1ee8c/core/src/main/java/jenkins/model/Jenkins.java#L5533-L5534