Make custom console respect custom params #1250

chmouel · 2023-04-25T10:20:00Z

Making it easier to consume between controller and watcher
Reworked most tests along the way and some refactor cleaning

Signed-off-by: Chmouel Boudjnah chmouel@redhat.com

Changes

Submitter Checklist

♽ Run make test lint before submitting a PR (ie: with pre-commit, no need to waste CPU cycle on CI
📖 If you are adding a user facing feature or make a change of the behavior, please verify that you have documented it
🧪 100% coverage is not a target but most of the time we would rather have a unit test if you make a code change.
🎁 If that's something that is possible to do please ensure to check if we can add a e2e test.
🔎 If there is a flakiness in the CI tests then don't necessary ignore it, better get the flakyness fixed before merging or if that's not possible there is a good reason to bypass it. (token rate limitation may be a good reason to skip).

pipelines-as-code · 2023-04-25T12:45:56Z

Golang test coverage difference report

Coverage decreased by 0.11%. 🔔 Shame 🔔

Package report

package                                                                            before    after    delta
-------                                                                           -------  -------  -------
pkg/acl                                                                           100.00%  100.00%         
pkg/action                                                                         68.75%   68.75%         
pkg/adapter                                                                        72.41%   72.41%         
pkg/apis/features                                                                 100.00%  100.00%         
pkg/cli/info                                                                       88.23%   88.23%         
pkg/cli/prompt                                                                     55.38%   55.38%         
pkg/cli/status                                                                     95.23%   95.23%         
pkg/cli/webhook                                                                    49.40%   49.40%         
pkg/cmd/tknpac/bootstrap                                                            2.11%    2.11%         
pkg/cmd/tknpac/completion                                                          50.00%   50.00%         
pkg/cmd/tknpac/create                                                              44.14%   44.14%         
pkg/cmd/tknpac/describe                                                            46.31%   46.31%         
pkg/cmd/tknpac/generate                                                            62.20%   62.20%         
pkg/cmd/tknpac/info                                                                62.50%   62.50%         
pkg/cmd/tknpac/list                                                                46.47%   46.47%         
pkg/cmd/tknpac/resolve                                                             71.42%   71.42%         
pkg/cmd/tknpac/webhook                                                             52.47%   52.47%         
pkg/consoleui                                                                      84.12%   82.60%   -1.52%
pkg/customparams                                                                   92.64%        -     gone
pkg/events                                                                         73.33%   73.33%         
pkg/formatting                                                                     98.71%   98.71%         
pkg/git                                                                            84.84%   84.84%         
pkg/hub                                                                            90.62%   90.62%         
pkg/kubeinteraction                                                                52.50%   52.50%         
pkg/kubeinteraction/status                                                         77.27%   77.27%         
pkg/matcher                                                                        86.47%   86.47%         
pkg/params/clients                                                                 14.86%   14.86%         
pkg/params/settings                                                                79.48%   79.48%         
pkg/pipelineascode                                                                 80.85%   82.91%   +2.06%
pkg/provider                                                                       76.19%   76.19%         
pkg/provider/bitbucketcloud                                                        86.78%   86.78%         
pkg/provider/bitbucketserver                                                       88.61%   88.61%         
pkg/provider/gitea                                                                 34.02%   34.02%         
pkg/provider/github                                                                82.96%   82.96%         
pkg/provider/github/app                                                            78.33%   78.33%         
pkg/provider/gitlab                                                                86.31%   86.31%         
pkg/random                                                                        100.00%  100.00%         
pkg/reconciler                                                                     42.81%   42.19%   -0.62%
pkg/resolve                                                                        87.93%   87.93%         
pkg/secrets                                                                        92.85%   92.85%         
pkg/sort                                                                           50.60%   51.20%   +0.60%
pkg/sync                                                                           91.13%   91.13%         
pkg/templates                                                                     100.00%  100.00%         
pkg/webhook                                                                        22.22%   22.22%         
                                                                          total:   66.75%   66.64%   -0.11%

chmouel · 2023-04-26T12:16:32Z

cc @mramendi

savitaashture

Hi @chmouel I have reviewed and executed code locally

Below are observation

This is how i configured in pipelines-as-code cm

  custom-console-name: savitaconsole
  custom-console-url: https://testconsole.com
  custom-console-url-pr-details: https://testconsole.com/detail/{{ custom }}
  custom-console-url-pr-tasklog: https://testconsole.com/log/{{ custom }}

My repo CR has

  params:
  - name: custom
    value: value

When CI started it gave custom console link properly
https://testconsole.com/detail/value
But when CI succeed the task URL started pointing to Openshift Console again
ex:
https://console-openshift-console.apps.51d375a4ef5b7ed2d78a.hypershift.aws-2.ci.openshift.org/k8s/ns/article-pipelines/tekton.dev~v1beta1~PipelineRun/article-no-operation-run-kxznh/logs/noop-task

Is that because you did not pass params here https://github.com/openshift-pipelines/pipelines-as-code/blob/main/pkg/params/run.go#L109 ?

docs/content/docs/install/settings.md

pkg/customparams/customparams_test.go

savitaashture · 2023-04-27T07:20:47Z

pkg/customparams/cel.go

@@ -0,0 +1,74 @@
+package customparams


Can we add test for cel as well ?

pkg/params/settings/validation.go

savitaashture · 2023-04-27T07:26:59Z

pkg/pipelineascode/pipelineascode.go

@@ -64,6 +65,15 @@ func (p *PacRun) Run(ctx context.Context) error {
 		p.manager.Enable()
 	}

+	// set params for the console driver, only used for the custom console ones
+	cp := customparams.NewCustomParams(p.event, repo, p.run, p.k8int, p.eventEmitter)


I see this changes only for custom console

So do we need to check if custom console configurations are set or not

If not set then we should skip this entire part?

yeah maybe we can make it conditional, i was thinking there may be other console driver that may need that information in the future

but yeah we don't know which driver is set from the interface we could add this but that's fine to have it to setparams by default, it can be used in the future and maybe not worse the refacotring

pkg/pipelineascode/template.go

test/gitea_test.go

chmouel · 2023-04-27T08:46:26Z

Is that because you did not pass params here main/pkg/params/run.go#L109 ?

this is weird, it's the watcher who report the final status, and we set the params in the watcher as soon as we get a repo match, i need to test on openshift more often 🙃 cause it works on tekton/dashboard

savitaashture · 2023-04-27T10:12:10Z

this is weird, it's the watcher who report the final status, and we set the params in the watcher as soon as we get a repo match, i need to test on openshift more often cause it works on tekton/dashboard

I was thinking how e2e passing but realized that we don't run e2e on Openshift 😆

savitaashture · 2023-04-27T10:34:40Z

this is weird, it's the watcher who report the final status, and we set the params in the watcher as soon as we get a repo match, i need to test on openshift more often upside_down_face cause it works on tekton/dashboard

Re verified and now its giving correct task log URL 🤷‍♀️

chmouel · 2023-04-28T18:45:17Z

/retest

chmouel · 2023-04-28T19:13:16Z

we really need to fix that bug where when previously failing it doesn't clear the old status when there is a infra error or something

savitaashture · 2023-05-01T14:57:49Z

we really need to fix that bug where when previously failing it doesn't clear the old status when there is a infra error or something

Do we have an already created issue or Do we need to create one ?

docs/content/docs/install/settings.md

savitaashture · 2023-05-01T16:47:26Z

I think we can merge the PR once after addressing that doc issue

Making it easier to consume between controller and watcher Reworked most tests along the way and some refactor cleaning Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>

Add our own function to apply configmap setting and revert to original state automatically. Remove gitea_custom_console_dashboard_test test since we have this covered with params. Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>

savitaashture · 2023-05-02T07:01:08Z

🎉 🎉

mramendi · 2023-05-03T01:32:14Z

docs/content/docs/install/settings.md


- example: `https://mycorp.com/ns/{{ namespace }}/pipelinerun/{{ pr }}/logs/{{ task }}#{{ pod }}-{{ firstFailedStep }}`
+  example: `https://mycorp.com/ns/{{ namespace }}/pipelinerun/{{ pr }}/logs/{{ task }}#{{ pod }}-{{ firstFailedStep }}`

 ## Pipelines-As-Code Info

  There are a settings exposed through a config map for which any authenticated


also remove "for"

mramendi · 2023-05-03T01:33:30Z

docs/content/docs/install/settings.md


 ## Pipelines-As-Code Info

  There are a settings exposed through a config map for which any authenticated
-  user can access to know about the Pipeline as Code status.
+  user can access to know about the Pipeline as Code status. This Configmap
+  will be automatically created with the [OpenShift Pipelines Operator](https://docs.openshift.com/container-platform/latest/cicd/pipelines/understanding-openshift-pipelines.html)


I think it would be nice to mention the name of the ConfigMap, seeing as the user might want to access it?

chmouel force-pushed the custom-params-on-console branch 2 times, most recently from 0d6d3bb to 3a91290 Compare April 25, 2023 12:44

chmouel marked this pull request as ready for review April 25, 2023 12:45

chmouel requested a review from savitaashture April 25, 2023 18:20

savitaashture reviewed Apr 27, 2023

View reviewed changes

chmouel changed the title ~~Add custom params on console~~ Make custom console respect custom params Apr 27, 2023

chmouel force-pushed the custom-params-on-console branch from 3a91290 to 60de549 Compare April 27, 2023 09:46

chmouel force-pushed the custom-params-on-console branch from 9649370 to 39a0858 Compare April 28, 2023 17:38

savitaashture reviewed May 1, 2023

View reviewed changes

docs/content/docs/install/settings.md Outdated Show resolved Hide resolved

savitaashture reviewed May 1, 2023

View reviewed changes

docs/content/docs/install/settings.md Outdated Show resolved Hide resolved

chmouel added 2 commits May 2, 2023 08:20

Move customparams feature to its own package

64b07b1

Making it easier to consume between controller and watcher Reworked most tests along the way and some refactor cleaning Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>

Make custom console respect custom params

780f3e2

Add our own function to apply configmap setting and revert to original state automatically. Remove gitea_custom_console_dashboard_test test since we have this covered with params. Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>

chmouel force-pushed the custom-params-on-console branch from ba359ba to 780f3e2 Compare May 2, 2023 06:22

savitaashture merged commit 9dd8fae into openshift-pipelines:main May 2, 2023

chmouel deleted the custom-params-on-console branch May 2, 2023 07:54

mramendi reviewed May 3, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make custom console respect custom params #1250

Make custom console respect custom params #1250

chmouel commented Apr 25, 2023

pipelines-as-code bot commented Apr 25, 2023 •

edited

Loading

chmouel commented Apr 26, 2023

savitaashture left a comment •

edited

Loading

savitaashture Apr 27, 2023

savitaashture Apr 27, 2023

chmouel Apr 27, 2023

chmouel Apr 27, 2023

chmouel commented Apr 27, 2023

savitaashture commented Apr 27, 2023

savitaashture commented Apr 27, 2023

chmouel commented Apr 28, 2023

chmouel commented Apr 28, 2023

savitaashture commented May 1, 2023

savitaashture commented May 1, 2023

savitaashture commented May 2, 2023

mramendi May 3, 2023

mramendi May 3, 2023

mramendi May 3, 2023

Make custom console respect custom params #1250

Make custom console respect custom params #1250

Conversation

chmouel commented Apr 25, 2023

Changes

Submitter Checklist

pipelines-as-code bot commented Apr 25, 2023 • edited Loading

Golang test coverage difference report

chmouel commented Apr 26, 2023

savitaashture left a comment • edited Loading

Choose a reason for hiding this comment

savitaashture Apr 27, 2023

Choose a reason for hiding this comment

savitaashture Apr 27, 2023

Choose a reason for hiding this comment

chmouel Apr 27, 2023

Choose a reason for hiding this comment

chmouel Apr 27, 2023

Choose a reason for hiding this comment

chmouel commented Apr 27, 2023

savitaashture commented Apr 27, 2023

savitaashture commented Apr 27, 2023

chmouel commented Apr 28, 2023

chmouel commented Apr 28, 2023

savitaashture commented May 1, 2023

savitaashture commented May 1, 2023

savitaashture commented May 2, 2023

mramendi May 3, 2023

Choose a reason for hiding this comment

mramendi May 3, 2023

Choose a reason for hiding this comment

mramendi May 3, 2023

Choose a reason for hiding this comment

pipelines-as-code bot commented Apr 25, 2023 •

edited

Loading

savitaashture left a comment •

edited

Loading