feat(task): Add capability to the slack task to dump file content and submodule git log #1215
Conversation
|
@pastequo, have you considered using Trusted Artifacts? This is how we safely allow custom Tasks to be included in the Pipeline. See the |
|
@lcarva Would you recommend using trusted artifacts over this contribution ? |
It's up to you. If it makes sense to have this Task here so others can use, then by all means let's go with that approach. We have found that a lot of times, users have specific operations they want to perform during a build pipeline, e.g. run unit tests. Those things don't necessarily lead to a Task that can be used by others. The Trusted Artifacts approach addresses this. Also, Trusted Artifacts is going to be the default for build pipelines in the near future. You may want to use it regardless, e.g. PVCs are not required. |
pastequo
force-pushed
the
feat/detailled-slack-notif
branch
from
1d623fa to
4ca90e6
Compare
|
IMHO I think it makes sense so have this enhancement here. If I'm not convincing enough I will try the Trusted Artifact method |
|
/ok-to-test |
pastequo
force-pushed
the
feat/detailled-slack-notif
branch
4 times, most recently
from
dca96b0 to
a7dbebe
Compare
|
I feel like those yaml linter error are not coming from my changes. Am I wrong ? |
pastequo
force-pushed
the
feat/detailled-slack-notif
branch
6 times, most recently
from
408cac8 to
326806d
Compare
pastequo
force-pushed
the
feat/detailled-slack-notif
branch
3 times, most recently
from
b86aaf6 to
7315372
Compare
task/slack-webhook-notification/0.1/slack-webhook-notification.yaml
Outdated
Show resolved
Hide resolved
| url=$(git config -f .gitmodules --get submodule."${name}".url) | ||
|
|
||
| current_commit=$(git -C "${path}" rev-parse HEAD) | ||
| previous_commit=$(git diff HEAD~1 "${path}" | grep commit | head -n 1 | awk '{print $3;}') |
There was a problem hiding this comment.
Will this only notice submodule changes if they happened in the latest commit? Is that the intention?
There was a problem hiding this comment.
TBH I don't know to detect efficiently if the submodule was updated. So I assume if it was, the merge commit will got it and then, in this case, 1 commit is enough. It's not perfect but I don't see an easy way to get this information
There was a problem hiding this comment.
Hmm, so it will work correctly if
- this runs in a push pipeline and the repo uses merge commits or
- updates to submodules are done in separate PRs, all submodules are updated in the same commit
That's probably fine, let's just document the shortcomings in the task description
task/slack-webhook-notification/0.1/slack-webhook-notification.yaml
Outdated
Show resolved
Hide resolved
task/slack-webhook-notification/0.1/slack-webhook-notification.yaml
Outdated
Show resolved
Hide resolved
pastequo
force-pushed
the
feat/detailled-slack-notif
branch
4 times, most recently
from
7c608e0 to
fbf4b09
Compare
|
/retest-required |
pastequo
force-pushed
the
feat/detailled-slack-notif
branch
from
fbf4b09 to
6134660
Compare
task/slack-webhook-notification/0.1/slack-webhook-notification.yaml
Outdated
Show resolved
Hide resolved
pastequo
force-pushed
the
feat/detailled-slack-notif
branch
from
6134660 to
ae0cce2
Compare
| @@ -8,4 +8,6 @@ Sends message to slack using incoming webhook | |||
| |message|Message to be sent||true| | |||
| |secret-name|Secret with least one key where value is webhook URL for slack. eg. oc create secret generic my-secret --from-literal team1=https://hooks.slack.com/services/XXX/XXXXXX --from-literal team2=https://hooks.slack.com/services/YYY/YYYYYY |slack-webhook-notification-secret|false| | |||
| |key-name|Key in the key in secret which contains webhook URL for slack.||true| | |||
| |files|List of file to dump. The content will be added to the message.|[]|false| | |||
| |submodules|List of submodules name to dump. Git log since previous submodule commit will be added to the message. The previous submodule commit is found by looking at the previous commit in the repository that declares the submodules.|[]|false| | |||
There was a problem hiding this comment.
nitpick: it's better to update this in the yaml so that the generate-readme.sh script doesn't remove this the next time someone remembers to use it
… submodule git log
pastequo
force-pushed
the
feat/detailled-slack-notif
branch
from
ae0cce2 to
13969ea
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
For RHEL-AI, each time there is a new build, I'm receiving requests to explain what is in and what's not.
I figure out that adding some configuration file content that is relevant to the build + some git history about upstream repo (a submodule) + some links most people are able to work. So I created a custom slack notification task for that, an inline task.
Since it's inline, it's not trusted and make the release pipeline fail. After some iteration with @ralphbean, he convinces me that it would be better to remove that custom task rather than adding an exception.
This is my humble attempt to make my task generic so that it can be reused by other