Add subcommand to upgrade Updatecli manifest

[olblak]

Allow to update Updatecli manifest between Updatecli version

Fix #618

This pullrequest is an attempt to allow upgrading updatecli manifest to the latest version.
This required the following changes

• Refactor the package config
• Allow to disable configuration templating
• Allow to write back a manifest to disk
• Ensure we don't downgrade manifest version
• Add version check between updatecli version and manifest version

Test

To test this pull request, you can run the following commands:

go build -o bin/updatecli . && ./bin/updatecli manifest upgrade --config examples/updatecli.d

Additional Information

Tradeoff

Because we load the configuration in memory, some fields are automatically added such as pipelineId, we need to identify those that should not be include

Potential improvement

• Run updatecli show without templating
• Rename updatecli show to updatecli manifest show for more consistency with updatecli manifeste upgrade
• Clarify the different between config title and config name
• Due due to the config package refactoring, we should be able to easily add other data file such as json
• Deprecated extension .tpl

[olblak]

I added the tag yaml:,omitempty to every spec fields otherwise when we upgrade a manifest, it automatically adds those fields with an empty value.

Instead of having

targets:
    default:
        name: jenkins
        kind: jenkins

we get

targets:
    default:
        disableSourceInput: fales
        name: jenkins
        kind: jenkins
        spec:
         ...
        transfomers:
        ...

if we don't omitempty when marshalling yaml

[dduportal]

Well I found the discussion interesting but a bit confusing, what do you think of

* `updatecli show manifest`: Display in the console an updatecli manifest. Similar to the current command `udpatecli show` (and we deprecated `updatecli show`)

* `updatecli validate manifest upgrade`: Test that a manifest can be upgrade

* `updatecli validate manifest syntax`: Test using jsonschema that a manifest is validate

* `updatecli upgrade manifest`: Try to upgrade an updatecli manifest to the latest version available based on updatecli version

Putting the verb first is confusing for me (like the kubectl CLI). I clearly prefer updatecli manifest show, updatecli manifest upgrade and updatecli manifest validate.

But your proposal makes sense in the sens that check and validate could be confused.

We could either:

• Define a new verb (I'mt putting my prefered order but it can be inverted of course) such as updatecli manifest checkupgrade?
• Go back to your initial proposal with the --check flag for updatecli manifest upgrade
• (my "prefere") Change slighty the logic of the upgradecli manifest upgrade to map tools such as https://github.com/jenkinsci/plugin-installation-manager-tool#updating-plugins:

## Show the upgraded manifest on stdout: user can redirect the stdout to whatever source they want
$ updatecli manifest upgrade  
## Upgrade the manifest file in place (instead of stdout)
$ updatecli manifest upgrade --write

[olblak]

putting the verb first is confusing for me

I am really not attached to the order so we can go with your suggestion

• updatecli manifest show: Display in the console an updatecli manifest. Similar to the current command udpatecli show (and we deprecated updatecli show)

• updatecli manifest validate: Test using jsonschema that a manifest is valid

• updatecli manifest upgrade: Try to upgrade an updatecli manifest to the latest version available based on updatecli version

updatecli manifest upgrade --write

I like very much your suggestion to display on stdout by default unless we specify a flag
I prefer the short version -i with the long version in-place similarly to "sops"

Go back to your initial proposal with the --check flag for updatecli manifest upgrade

What do you think of using the parameter --dry-run such as updatecli manifest upgrade --dry-run

[olblak]

putting the verb first is confusing for me

I am really not attached to the order so we can go with your suggestion

* `updatecli manifest show`: Display in the console an updatecli manifest. Similar to the current command `udpatecli show` (and we deprecated `updatecli show`)

* `updatecli manifest validate`: Test using jsonschema that a manifest is valid

* `updatecli manifest upgrade`: Try to upgrade an updatecli manifest to the latest version available based on updatecli version

updatecli manifest upgrade --write

I like very much your suggestion to display on stdout by default unless we specify a flag I prefer the short version -i with the long version in-place similarly to "sops"

Go back to your initial proposal with the --check flag for updatecli manifest upgrade

What do you think of using the parameter --dry-run such as updatecli manifest upgrade --dry-run

I just realise that --in-place and --dry-run fill the same purpose. I am dropping --dry-run

[olblak]

I think the PR is ready to be merged

The new command is ``updatecli manifest upgrade`

Usage:
  updatecli manifest upgrade [flags]

Flags:
  -c, --config string   Sets config file or directory. (default: './updatecli.yaml') (default "./updatecli.yaml")
  -h, --help            help for upgrade
  -i, --in-place        Write updated Updatecli manifest back to the same file instead of stdout

Global Flags:
      --debug   Debug Output

The dry-run is the default behavior.

I did some testing with the example directory

++++++++++++++++++++
+ MANIFEST UPGRADE +
++++++++++++++++++++

Loading Pipeline "examples/updatecli.d/nginx.yaml"
WARNING: The directive 'scm' for the target["jenkinsioNginxDigest"] is now deprecated. Please use the new top level scms syntax
⚠ Updatecli manifest change required
--- examples/updatecli.d/nginx.yaml(old)
+++ examples/updatecli.d/nginx.yaml(updated)
@@ -1,23 +1,36 @@
+name: NGINX.YAML
+pipelineid: 5d2ef3f73e3c99cb9d6470470fa613b71156b3a9b229e2b62feba3f4f1216559
+pullrequests:
+    target_jenkinsioNginxDigest:
+        kind: github
+        spec: {}
+        scmid: target_jenkinsioNginxDigest
+        targets:
+            - jenkinsioNginxDigest
+scms:
+    target_jenkinsioNginxDigest:
+        kind: github
+        spec:
+            branch: master
+            email: update-bot@olblak.com
+            owner: jenkins-infra
+            repository: charts
+            token: x
+            user: update-bot
+            username: olblak
 sources:
-  lastDockerDigest:
-    kind: dockerdigest
-    name: "Get latest nginx:1.17 from dockerhub"
-    spec:
-      image: "library/nginx"
-      tag: "1.17"
+    lastDockerDigest:
+        name: Get latest nginx:1.17 from dockerhub
+        kind: dockerdigest
+        spec:
+            image: library/nginx
+            tag: "1.17"
 targets:
-  jenkinsioNginxDigest:
-    name: "Jenkins.io nginx"
-    kind: yaml
-    spec:
-      file: "charts/jenkins.io/values.yaml"
-      key: image.tag
-    scm:
-      github:
-        user: "update-bot"
-        email: "update-bot@olblak.com"
-        owner: "jenkins-infra"
-        repository: "charts"
-        token: "x"
-        username: "olblak"
-        branch: "master"
+    jenkinsioNginxDigest:
+        name: Jenkins.io nginx
+        kind: yaml
+        spec:
+            file: charts/jenkins.io/values.yaml
+            key: image.tag
+        scmid: target_jenkinsioNginxDigest
+        sourceid: lastDockerDigest

I run the new command in the example directory, and I spot few configuration errors.

[dduportal]

Tested on https://github.com/jenkins-infra/kubernetes-management/ with a recursive command update and it seems to work very well:

• Checkouted to an old commit (dd34d184efea9cf10900019feb78410b9813aa70)
• Checked the default behavior with updatecli-next manifest upgrade --config updatecli/updatecli.d : showed a LOT of possible changes
• Updated with the --in-place flag and ran the previous command again, which showed no update required!

I saw 2 things, not blocking the merge (subsequent PRs):

• I saw the kind: helmChart instructions not being updated to lower case
• The exit code of updatecli-next manifest upgrade --config updatecli/updatecli.d (dry run) is zero wether there are changes or not. A nice improvement would be to add a flag such as https://www.terraform.io/cli/commands/plan#detailed-exitcode to indicate through the exit code if a change is required or not (to ease pipeline processes)

Great feature buddy!

[olblak]

Thanks very much testing.

The exit code of updatecli-next manifest upgrade --config updatecli/updatecli.d (dry run) is zero wether there are changes or not. A nice improvement would be to add a flag such as https://www.terraform.io/cli/commands/plan#detailed-exitcode to indicate through the exit code if a change is required or not (to ease pipeline process

Indeed that's a great suggestion and I think that it should be in a different PR so we can also handle the other commands
Meanwhile I am adding the "report" such

++++++++++++++++++++
+ MANIFEST UPGRADE +
++++++++++++++++++++

Loading Pipeline "examples/updatecli.d/nginx.yaml"
WARNING: The directive 'scm' for the target["jenkinsioNginxDigest"] is now deprecated. Please use the new top level scms syntax
WARNING: The directive 'scm' for the target["jenkinsioNginxDigest"] is now deprecated. Please use the new top level scms syntax
✔ No Updatecli manifest change required



Summary
===========
Manifest(s) upgrade:
  * Changed:	0
  * Failed:	0
  * Skipped:	0
  * Succeeded:	1
  * Total:	1

Which should already highlight changes

Before merging, I sport some bugs that I need to fix

[olblak]

saw the kind: helmChart instructions not being updated to lower case

This is because we don't validate that the kind value is lowercase. I am currently adding this to the current PR as