bugfix ks 5193 (#888)

* add updatePipelineConfigXml

* remove timerTrigger if disable from console

pkg/client/devops/jclient/projectPipeline.go

@@ -82,7 +82,6 @@ func (j *JenkinsClient) DeleteProjectPipeline(projectID string, pipelineID strin
 
 // UpdateProjectPipeline updates pipeline
 func (j *JenkinsClient) UpdateProjectPipeline(projectID string, pipeline *devopsv1alpha3.Pipeline) (string, error) {
-	// TODO: Update a pipeline
 	return j.jenkins.UpdateProjectPipeline(projectID, pipeline)
 }
 

pkg/client/devops/jenkins/constants.go

@@ -35,6 +35,35 @@ const (
 	PROJECT_ROLE = "projectRoles"
 )
 
+const (
+	StringNull = ""
+
+	FlowTag                 = "flow-definition"
+	PropertiesTag           = "properties"
+	DisableConcurrentJobTag = "org.jenkinsci.plugins.workflow.job.properties.DisableConcurrentBuildsJobProperty"
+	BuildDiscarderTag       = "jenkins.model.BuildDiscarderProperty"
+	StrategyTag             = "strategy"
+	DaysToKeepTag           = "daysToKeep"
+	NumToKeepTag            = "numToKeep"
+	ArtiDaysToKeepTag       = "artifactDaysToKeep"
+	ArtiNumToKeepTag        = "artifactNumToKeep"
+
+	ParamDefiPropTag = "hudson.model.ParametersDefinitionProperty"
+	ParamDefiTag     = "parameterDefinitions"
+
+	PipelineTriggersJobTag = "org.jenkinsci.plugins.workflow.job.properties.PipelineTriggersJobProperty"
+	TriggersTag            = "triggers"
+	TimerTriggerTag        = "hudson.triggers.TimerTrigger"
+
+	DefinitionTag = "definition"
+	ClassKey      = "class"
+	PluginKey     = "plugin"
+	ScriptTag     = "script"
+	SandboxTag    = "sandbox"
+	AuthTokenTag  = "authToken"
+	DisabledTag   = "disabled"
+)
+
 // ParameterTypeMap aims for simplifying representation of parameter definition type.
 var ParameterTypeMap = map[string]string{
 	"hudson.model.StringParameterDefinition":   "string",

pkg/client/devops/jenkins/pipeline_internal.go

@@ -72,7 +72,7 @@ func createPipelineConfigXml(pipeline *devopsv1alpha3.NoScmPipeline) (string, er
 		strategy.CreateElement("artifactNumToKeep").SetText("-1")
 	}
 	if pipeline.Parameters != nil {
-		appendParametersToEtree(properties, pipeline.Parameters)
+		replaceParametersInEtree(properties, pipeline.Parameters)
 	}
 
 	// create trigger xml structure
@@ -111,6 +111,87 @@ func createPipelineConfigXml(pipeline *devopsv1alpha3.NoScmPipeline) (string, er
 	return replaceXmlVersion(stringXml, "1.0", "1.1"), err
 }
 
+func updatePipelineConfigXml(config string, pipeline *devopsv1alpha3.NoScmPipeline) (string, error) {
+	config = replaceXmlVersion(config, "1.1", "1.0")
+	doc := etree.NewDocument()
+	err := doc.ReadFromString(config)
+	if err != nil {
+		return "", err
+	}
+	flow := doc.SelectElement(FlowTag)
+	// ------------------------------------------------
+	// update properties
+	properties := flow.SelectElement(PropertiesTag)
+	if pipeline.DisableConcurrent {
+		addOrUpdateElement(properties, DisableConcurrentJobTag, StringNull)
+	} else {
+		removeChildElement(properties, DisableConcurrentJobTag)
+	}
+
+	if pipeline.Discarder != nil {
+		var discarder, strategy *etree.Element
+		discarder = addOrUpdateElement(properties, BuildDiscarderTag, StringNull)
+		strategy = addOrUpdateElement(discarder, StrategyTag, StringNull)
+
+		replaceAttr(strategy, ClassKey, "hudson.tasks.LogRotator")
+		addOrUpdateElement(strategy, DaysToKeepTag, pipeline.Discarder.DaysToKeep)
+		addOrUpdateElement(strategy, NumToKeepTag, pipeline.Discarder.NumToKeep)
+		addOrUpdateElement(strategy, ArtiDaysToKeepTag, "-1")
+		addOrUpdateElement(strategy, ArtiNumToKeepTag, "-1")
+	}
+
+	if pipeline.Parameters != nil { // overwrite parameters
+		replaceParametersInEtree(properties, pipeline.Parameters)
+	} else {
+		removeChildElement(properties, ParamDefiPropTag)
+	}
+
+	// update triggers xml structure
+	var pipelineTriggerEle, triggersEle *etree.Element
+	pipelineTriggerEle = addOrUpdateElement(properties, PipelineTriggersJobTag, StringNull)
+	triggersEle = addOrUpdateElement(pipelineTriggerEle, TriggersTag, StringNull)
+
+	if pipeline.TimerTrigger != nil {
+		timerTriggerEle := addOrUpdateElement(triggersEle, TimerTriggerTag, StringNull)
+		addOrUpdateElement(timerTriggerEle, "spec", pipeline.TimerTrigger.Cron)
+	} else {
+		removeChildElement(triggersEle, TimerTriggerTag)
+	}
+
+	if pipeline.GenericWebhook != nil {
+		// TODO issue: if support GenericWebhook in console, need to delete GenericWebhook tag when pipeline.GenericWebhook is nil;
+		triggers.CreateGenericWebhookXML(triggersEle, pipeline.GenericWebhook)
+	}
+
+	// ------------------------------------------------
+	// replace definition(all fields could update from console)
+	removeChildElement(flow, DefinitionTag)
+	pipelineDefine := flow.CreateElement(DefinitionTag)
+	pipelineDefine.CreateAttr(ClassKey, "org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition")
+	pipelineDefine.CreateAttr(PluginKey, "workflow-cps")
+	pipelineDefine.CreateElement(ScriptTag).SetText(pipeline.Jenkinsfile)
+	pipelineDefine.CreateElement(SandboxTag).SetText("true")
+
+	// ------------------------------------------------
+	// update others
+	if flow.SelectElement(TriggersTag) == nil {
+		flow.CreateElement(TriggersTag)
+	}
+	// TODO issue: if support RemoteTrigger in console, need to delete GenericWebhook tag when pipeline.GenericWebhook is nil;
+	if pipeline.RemoteTrigger != nil {
+		addOrUpdateElement(flow, AuthTokenTag, pipeline.RemoteTrigger.Token)
+	}
+	addOrUpdateElement(flow, DisabledTag, "false")
+
+	// format xml string
+	doc.Indent(2)
+	stringXml, err := doc.WriteToString()
+	if err != nil {
+		return "", err
+	}
+	return replaceXmlVersion(stringXml, "1.0", "1.1"), err
+}
+
 func parsePipelineConfigXml(config string) (*devopsv1alpha3.NoScmPipeline, error) {
 	pipeline := &devopsv1alpha3.NoScmPipeline{}
 	config = replaceXmlVersion(config, "1.1", "1.0")
@@ -177,13 +258,21 @@ func parsePipelineConfigXml(config string) (*devopsv1alpha3.NoScmPipeline, error
 	return pipeline, nil
 }
 
-func appendParametersToEtree(properties *etree.Element, parameters []devopsv1alpha3.ParameterDefinition) {
-	parameterDefinitions := properties.CreateElement("hudson.model.ParametersDefinitionProperty").
-		CreateElement("parameterDefinitions")
+func replaceParametersInEtree(properties *etree.Element, parameters []devopsv1alpha3.ParameterDefinition) {
+	var paramDefiPropsE, paramDefiE *etree.Element
+	if paramDefiPropsE = properties.SelectElement(ParamDefiPropTag); paramDefiPropsE == nil {
+		paramDefiE = properties.CreateElement(ParamDefiPropTag).CreateElement(ParamDefiTag)
+	} else {
+		if paramDefiE = paramDefiPropsE.SelectElement(ParamDefiTag); paramDefiE != nil {
+			paramDefiPropsE.RemoveChild(paramDefiE)
+		}
+		paramDefiE = paramDefiPropsE.CreateElement(ParamDefiTag)
+	}
+
 	for _, parameter := range parameters {
 		for className, typeName := range ParameterTypeMap {
 			if typeName == parameter.Type {
-				paramDefine := parameterDefinitions.CreateElement(className)
+				paramDefine := paramDefiE.CreateElement(className)
 				paramDefine.CreateElement("name").SetText(parameter.Name)
 				paramDefine.CreateElement("description").SetText(parameter.Description)
 				switch parameter.Type {
@@ -510,3 +599,27 @@ func toCrontab(millis int64) string {
 	return "H H * * *"
 
 }
+
+func addOrUpdateElement(parent *etree.Element, tag, text string) *etree.Element {
+	var e *etree.Element
+	if e = parent.SelectElement(tag); e == nil {
+		e = parent.CreateElement(tag)
+	}
+	if text != "" {
+		e.SetText(text)
+	}
+	return e
+}
+
+func replaceAttr(e *etree.Element, key, value string) *etree.Element {
+	e.RemoveAttr(key)
+	e.CreateAttr(key, value)
+	return e
+}
+
+func removeChildElement(parent *etree.Element, childTag string) *etree.Element {
+	if e := parent.SelectElement(childTag); e != nil {
+		parent.RemoveChild(e)
+	}
+	return parent
+}

pkg/client/devops/jenkins/project_pipeline.go

@@ -93,29 +93,31 @@ func (j *Jenkins) DeleteProjectPipeline(projectId string, pipelineId string) (st
 func (j *Jenkins) UpdateProjectPipeline(projectId string, pipeline *devopsv1alpha3.Pipeline) (string, error) {
 	switch pipeline.Spec.Type {
 	case devopsv1alpha3.NoScmPipelineType:
-		config, err := createPipelineConfigXml(pipeline.Spec.Pipeline)
+		job, err := j.GetJob(pipeline.Name, projectId)
 		if err != nil {
-			return "", restful.NewError(http.StatusInternalServerError, err.Error())
+			return "", restful.NewError(devops.GetDevOpsStatusCode(err), err.Error())
 		}
-
-		job, err := j.GetJob(pipeline.Name, projectId)
-
+		config, err := job.GetConfig()
 		if err != nil {
 			return "", restful.NewError(devops.GetDevOpsStatusCode(err), err.Error())
 		}
 
-		err = job.UpdateConfig(config)
+		updatedConfig, err := updatePipelineConfigXml(config, pipeline.Spec.Pipeline)
 		if err != nil {
-			return "", restful.NewError(devops.GetDevOpsStatusCode(err), err.Error())
+			klog.Errorf("%+v", err)
+			return "", restful.NewError(http.StatusInternalServerError, err.Error())
 		}
 
+		err = job.UpdateConfig(updatedConfig)
+		if err != nil {
+			return "", restful.NewError(devops.GetDevOpsStatusCode(err), err.Error())
+		}
 		return pipeline.Name, nil
-	case devopsv1alpha3.MultiBranchPipelineType:
 
+	case devopsv1alpha3.MultiBranchPipelineType:
 		config, err := createMultiBranchPipelineConfigXml(projectId, pipeline.Spec.MultiBranchPipeline)
 		if err != nil {
 			klog.Errorf("%+v", err)
-
 			return "", restful.NewError(http.StatusInternalServerError, err.Error())
 		}
 

pkg/client/devops/jenkins/triggers/genericwebhook.go

@@ -29,6 +29,10 @@ func CreateGenericWebhookXML(parent *etree.Element, webhook *v1alpha3.GenericWeb
 		return
 	}
 
+
+	if gtE := parent.SelectElement("org.jenkinsci.plugins.gwt.GenericTrigger"); gtE != nil {
+		parent.RemoveChild(gtE)
+	}
 	ele = parent.CreateElement("org.jenkinsci.plugins.gwt.GenericTrigger")
 
 	ele.CreateElement("spec")