Support the event watcher in the app config

[knanao]

What this PR does / why we need it:
This allows settings related to EventWatcher to be defined in the application configuration file as well as ./pipe.
Both configurations of the EventWatcher under ./pipe and under the application configuration file are referred to.

NOTE: In the future ./pipe will be Deprecated.

apiVersion: pipecd.dev/v1beta1
kind: ApplicationKind
spec:
  name: foo
  labels:
    team: bar
  eventWatcher:
    - matcher:
        name: helloworld-image-update
        labels:
          env: dev
          appName: helloworld
      handler:
        type: GIT_UPDATE
        config:
          replacements:
            - file: deployment.yaml
              yamlField: $.spec.template.spec.containers[0].image
          commitMessage: Update values by Event watcher

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

NONE

[nghialv]

@knanao 👍

@knanao @khanhtc1202 I think firstly we should list all available configuration patterns to find the suitable one.

Let's comment all patterns you can think of.

apiVersion: pipecd.dev/v1beta1
kind: ApplicationKind
spec:
  name: foo
  labels:
    team: bar
  events:
    - name: helloworld-image-update
      labels:
        env: dev
        appName: helloworld
      pushHandler:
        commitMessage: Update values by Event watcher
        replacements:
          - file: deployment.yaml
            yamlField: $.spec.template.spec.containers[0].image

apiVersion: pipecd.dev/v1beta1
kind: ApplicationKind
spec:
  name: foo
  labels:
    team: bar
  eventWatcher:
    - event:
        name: helloworld-image-update
        labels:
          env: dev
          appName: helloworld
      handler:
        gitUpdate:
          replacements:
            - file: deployment.yaml
              yamlField: $.spec.template.spec.containers[0].image
          commitMessage: Update values by Event watcher

apiVersion: pipecd.dev/v1beta1
kind: ApplicationKind
spec:
  name: foo
  labels:
    team: bar
  eventWatcher:
    - event:
        name: helloworld-image-update
        labels:
          env: dev
          appName: helloworld
      gitPushHandler:
        replacements:
          - file: deployment.yaml
            yamlField: $.spec.template.spec.containers[0].image
        commitMessage: Update values by Event watcher

apiVersion: pipecd.dev/v1beta1
kind: ApplicationKind
spec:
  name: foo
  labels:
    team: bar
  eventWatcher:
    - event:
        name: helloworld-image-update
        labels:
          env: dev
          appName: helloworld
      handler:
        type: GIT_UPDATE
        config:
          replacements:
            - file: deployment.yaml
              yamlField: $.spec.template.spec.containers[0].image
          commitMessage: Update values by Event watcher

[knanao]

@nghialv
Thank you for your suggestions and that's a nice idea.
And I wanna vote for a 1 or 3 style because it's simple compared to the other.
And I wanna share the other suggestions like this.
WDYT? @khanhtc1202

apiVersion: pipecd.dev/v1beta1
kind: ApplicationKind
spec:
  name: foo
  labels:
    team: bar
  eventWatcher:
    - matcher:
        name: helloworld-image-update
        labels:
          env: dev
          appName: helloworld
      handler:
        gitUpdate:
          replacements:
            - file: deployment.yaml
              yamlField: $.spec.template.spec.containers[0].image
          commitMessage: Update values by Event watcher

apiVersion: pipecd.dev/v1beta1
kind: ApplicationKind
spec:
  name: foo
  labels:
    team: bar
  eventWatcher:
    - matcher:
        name: helloworld-image-update
        labels:
          env: dev
          appName: helloworld
      handler:
        type: GIT_UPDATE
        config:
          replacements:
            - file: deployment.yaml
              yamlField: $.spec.template.spec.containers[0].image
          commitMessage: Update values by Event watcher

apiVersion: pipecd.dev/v1beta1
kind: ApplicationKind
spec:
  name: foo
  labels:
    team: bar
  eventWatcher:
    - matcher:
        name: helloworld-image-update
        labels:
          env: dev
          appName: helloworld
      gitUpdator:
        replacements:
          - file: deployment.yaml
             yamlField: $.spec.template.spec.containers[0].image
        commitMessage: Update values by Event watcher

apiVersion: pipecd.dev/v1beta1
kind: ApplicationKind
spec:
  name: foo
  labels:
    team: bar
  eventWatcher:
    - matchers:
        - name: helloworld-image-update
           labels:
             env: dev
             appName: helloworld
      handler:
         type: GIT_UPDATE
         config:
           replacements:
             - file: deployment.yaml
                yamlField: $.spec.template.spec.containers[0].image
           commitMessage: Update values by Event watcher

[knanao]

Finally, we decided it into the config pattern of 6.
so let me apply it.

[knanao]

@nghialv @khanhtc1202
I applied the above discussion to this PR.
And I changed eventWatcher: to eventWatchers: because that is slice elements.
So the final configuration is as follows.

apiVersion: pipecd.dev/v1beta1
kind: ApplicationKind
spec:
  name: foo
  labels:
    team: bar
  eventWatchers:
    - matcher:
        name: helloworld-image-update
        labels:
          env: dev
          appName: helloworld
      handler:
        type: GIT_UPDATE
        config:
          replacements:
            - file: deployment.yaml
              yamlField: $.spec.template.spec.containers[0].image
          commitMessage: Update values by Event watcher

[khanhtc1202]

how about eventsWatcher instead of eventWatchers 🤔

[knanao]

Since the elements of the event watcher are arrays,
I selected the plural form, but actually, the singular form seems to be better.
Thank you🙌

[khanhtc1202]

If the configuration file of the EventWatcher exists under ./pipe for a specific repository, refer to it, but if it does not exist, refer to the settings of the target Application configuration file.

I'm a bit concerned about this. We want to make EventWatcher configuration under .pipe/ directory deprecated, so I think instead of making the configuration in .piped/ be referred first (with higher priority), the configuration in the application configuration should have a higher priority. And if both exist, only application configuration eventsWatcher should be applied, we only apply the EventWatcher config under .pipe/ dir if it's the only one that existed. WDYT?

[knanao]

@khanhtc1202
Thanks for your reviews.

so I think instead of making the configuration in .piped/ be referred first (with higher priority), the configuration in the application configuration should have a higher priority.

If we had to prioritize, I would agree totally.

And if both exist, only application configuration eventsWatcher should be applied, we only apply the EventWatcher config under .pipe/ dir if it's the only one that existed.

The reason for this implementation without prioritization is to allow for migration from one environment to another.
If we have priorities, the user has to replace everything in a repository with the application config at once. That doesn't sound very user-friendly like a large-scale project.
(Checking for duplicates at each event would be nice, but that seems a bit complicated.)

In order to be able to migrate gradually from one environment to another, it is necessary to temporarily make a state in which settings can be entered in both .pipe/ and application config hence I did like this.

But I would like to hear from guys.

[khanhtc1202]

I get your point 👍 But still think if we allow both places, duplicates may occur 🤔 My idea is to allow only one (in application config or in .pipe/ dir) users will be aware of the change and migrate it while adding a new eventWatcher to their application's configuration.
But, image myself with more than one hundred applications, it would be a pain 😓

[knanao]

But still think if we allow both places, duplicates may occur

I think the duplicates won't occur in this implementation because it is processed sequentially.
This means that the event which is already handled is never handled by this.

But, image myself with more than one hundred applications, it would be a pain 😓

yeah, it's not user-friendly actually hence I adapted like that.

wdyt? @nghialv

[nghialv]

@knanao @khanhtc1202 I also think having a phase where both configuration ways are supported is more user-friendly.
As you know, the old configuration way is being used widely by some large organizations. More than that, they have many sub-teams where each team owns and manages a group of applications. Force all the teams to migrate to the new configuration in the same PR is not realistic. (Since the old configuration is Piped level, not application level). So if possible, we should give them a long-enough period for the migration process.

[nghialv]

@knanao Thank you. Looks fine. Just added a nit.

[nghialv]

👍

[khanhtc1202]

Here you go 👍