Update Go, Kubernetes and other dependencies #582

pkosiec · 2022-05-19T13:42:43Z

ISSUE TYPE

Feature Pull Request

SUMMARY

Update Go to 1.18
Update Kubernetes-related dependencies to 0.24
Update other dependencies (such as Slack, Mattermost and Discord clients, and also fsnotify, logrus, go-github....)
- Use github.com/slack-go/slack (community-maintained fork of nlopes/slack) instead of nlopes/slack
Use Alpine 3.15 as a base image
~~Fix panic when a singleton Event is received~~

Resolves #587

TESTING

I've already tested Slack, Mattermost, and Discord. Unfortunately, I couldn't test other integrations, but for the others I bumped them just to the latest minor versions (to not take any breaking changes).

You can use my Docker image pkosiec/botkube:update-deps which I've built with the following commands:

make container-image
docker tag ghcr.io/infracloudio/botkube:v9.99.9-dev-amd64 pkosiec/botkube:update-deps
docker push pkosiec/botkube:update-deps

pkosiec · 2022-05-20T08:19:44Z

pkg/utils/utils.go

@@ -303,7 +302,7 @@ func InitResourceMap(conf *config.Config) {
 		AllowedKubectlVerbMap[r] = true
 	}

-	resourceList, err := DiscoveryClient.ServerResources()
+	_, resourceList, err := DiscoveryClient.ServerGroupsAndResources()


ServerResources was deprecated and removed in 1.24. For example, see comment from 1.22:
https://github.com/kubernetes/client-go/blob/v0.22.7/discovery/discovery_client.go#L98

pkosiec · 2022-05-20T08:20:55Z

pkg/utils/utils.go

@@ -212,7 +212,6 @@ func GetObjectMetaData(obj interface{}) metaV1.ObjectMeta {
 		Annotations:                unstructuredObject.GetAnnotations(),
 		OwnerReferences:            unstructuredObject.GetOwnerReferences(),
 		Finalizers:                 unstructuredObject.GetFinalizers(),
-		ClusterName:                unstructuredObject.GetClusterName(),


https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/object-meta/#Ignored

Deprecated: ClusterName is a legacy field that was always cleared by the system and never used; it will be removed completely in 1.25.

pkosiec · 2022-05-20T08:58:39Z

Hi @PrasadG193,
will you have time to take a look at this PR please? It would be great to have all dependencies up-to-date.

This is a base for #581 and also I will use it to prepare other PRs with improvements 🙂 Thanks a lot!

EDIT: I also rebased the pull request 🙂

pkosiec · 2022-05-20T09:19:51Z

pkg/events/events.go

@@ -124,7 +124,7 @@ func New(object interface{}, eventType config.EventType, resource, clusterName s
 		event.Action = eventObj.Action
 		event.TimeStamp = eventObj.LastTimestamp.Time
 		// Compatible with events.k8s.io/v1
-		if eventObj.LastTimestamp.IsZero() {
+		if eventObj.LastTimestamp.IsZero() && eventObj.Series != nil {


Now I can see that I fixed the same bug as was observed by the author of PR #577. I wanted to revert that change to allow #577 to be merged, but unfortunately #577 solution is invalid and doesn't fix that problem.

PrasadG193 · 2022-05-21T14:05:57Z

Hey @pkosiec,
This could be a breaking change. We were thinking about releasing BotKube v0.13.0 to announce Lark support. We can plan this for the next release. Let me know what you think?

pkosiec · 2022-05-24T08:46:18Z

Hi @PrasadG193, which part of this pull request do you consider as a breaking change?

Do you mean the Kubernetes-related dependencies (like client-go)? Between K8s 1.20 and 1.24 there were the following breaking changes in deprecated APIs: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#v1-22. As you can see, all the removed APIs have been deprecated since Kubernetes 1.8-1.19. So there shouldn't be any problems, and to avoid technical debt, it would be great to have the dependencies constantly updated.

When it comes to the integration-related dependencies (Discord, Slack, etc.) I bumped them to the latest minor versions, so don't worry, they shouldn't have any breaking changes if they all do proper SemVer tagging (from what I saw, they do).

That said, I think it would be great to do alpha/beta release before releasing a stable version and give time to the real users to report any issues they face.

And when it comes to 0.13 release, could you please hold off before releasing a new version? Along with @mszostok, we want to contribute quite a lot of features (see our GitHub Project we prepared: https://github.com/infracloudio/botkube/projects/10). So we were thinking about having feature-packed 0.13 to show the community this cool project lives on, and the development didn't stop 🚀 The Slack multichannel support could be a release theme, and the first alpha version could be released within a month. But, before we contribute such changes, we need to make sure there's no technical debt to be fully productive with implementing new features.

What do you think? Cheers! 🙂

pkosiec · 2022-05-25T07:16:37Z

PR rebased to the latest develop 🙂

PrasadG193 · 2022-05-25T07:16:37Z

Hey @pkosiec,
Sounds good. We wanted to do a release to announce Lark support. I am working on Lark support documentation and a few improvements. @mszostok Do you think we should announce Lark support in a separate release or clubbing that with 0.13.0 along with other features would make sense?

pkosiec · 2022-05-25T07:46:49Z

As far as I know, @mszostok will be AFK until tomorrow, but of course let's wait for his opinion 🙂 I think it would make sense to deliver Lark support along with other features, as, as you said, the Lark documentation is still in progres. In a result there will be more time to finalize the documentation without blocking some other features from merging 🙂 Later we could switch from git flow to GitHub flow and have dedicated release branches, to make sure we can support multiple versions.

@PrasadG193 Anyway, apart from the release, does my explanation around dependencies bump / breaking changes make sense? Is it OK for you to have this PR merged soon, regardless which features 0.13 will bring? Thanks! 🙂

PrasadG193 · 2022-05-25T07:50:16Z

Cool, sounds good to me. 🙂
Thank you!

.github/workflows/ci.yml

go.sum

PrasadG193

LGTM

##### ISSUE TYPE - Bug fix Pull Request ##### SUMMARY - Do not log full Slack event details in case of unmarshalling error This is an issue of the upstream library. While I prepared a workaround, I also prepared a simple change in `slack-go/slack` repo: slack-go/slack#1067 **NOTE:** This pull request depends on #582. Please merge #582 first, and I will rebase this one. Actual changes on this PR: 7813821 Fixes #579 ##### TESTING See the original issue for testing instruction: #579 You can use `pkosiec/botkube:slack-unmarshall-err` Docker image for testing the workaround. Once you run my image and try to reproduce the issue, you'll see in the logs: ``` ERRO[2022-05-19T16:00:57Z] Slack unmarshalling error: RTM Error: Received unmapped event "user_huddle_changed" ERRO[2022-05-19T16:00:57Z] Slack unmarshalling error: RTM Error: Received unmapped event "sh_room_join" ERRO[2022-05-19T16:01:00Z] Slack unmarshalling error: RTM Error: Received unmapped event "sh_room_leave" ```

##### ISSUE TYPE - Feature Pull Request ##### SUMMARY Add an option to push the BotKube image automatically on PR. It's alternative approach for #604. This PR will solve the problem with manual PR builds, e.g. we had that issue here: - #601 - #593 - #582 - #583 Example run: https://github.com/mszostok/botkube/runs/6714112689?check_suite_focus=true Fixes #590 To ensure that secrets won't be available for untrusted code, first we need to build the image and share it with the second job, which doesn't check out the untrusted code and can safely push an artifact to ghcr.io. The flow is as follows: ``` Job1: image build -> image save -> artifact upload Job2: artifact download -> image load -> image push ``` Job1—runs untrusted code but without write repo perms Job2—push built image with package write perms #### Security This article describes it well: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

pkosiec added the enhancement New feature or request label May 19, 2022

pkosiec mentioned this pull request May 19, 2022

Do not log full Slack event details in case of unmarshalling error #583

Merged

pkosiec commented May 20, 2022

View reviewed changes

pkosiec force-pushed the update-dependencies branch from 2ef85a0 to a988ea9 Compare May 20, 2022 09:02

pkosiec commented May 20, 2022

View reviewed changes

pkosiec mentioned this pull request May 20, 2022

Use golangci-lint and fix all found issues #588

Closed

pkosiec added 3 commits May 25, 2022 09:15

Update dependencies

1273551

Upgrade Go and Kubernetes dependencies

d5ccf0a

Use Alpine 3.15 as a base Docker image

ecda789

pkosiec force-pushed the update-dependencies branch from a988ea9 to d3266ec Compare May 25, 2022 07:16

PrasadG193 reviewed May 25, 2022

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

go.sum Show resolved Hide resolved

Update golint installation on CI

7902189

pkosiec force-pushed the update-dependencies branch from d3266ec to 7902189 Compare May 25, 2022 08:11

PrasadG193 approved these changes May 25, 2022

View reviewed changes

pkosiec added the ready-to-merge label May 25, 2022

mergify bot merged commit 40ade68 into kubeshop:develop May 25, 2022

pkosiec deleted the update-dependencies branch May 25, 2022 08:23

This was referenced Jun 1, 2022

Add option to build&push image on PR via label #604

Closed

Add option to build&push image on PR automatically #611

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Go, Kubernetes and other dependencies #582

Update Go, Kubernetes and other dependencies #582

pkosiec commented May 19, 2022 •

edited

Loading

pkosiec May 20, 2022

pkosiec May 20, 2022

pkosiec commented May 20, 2022 •

edited

Loading

pkosiec May 20, 2022

PrasadG193 commented May 21, 2022

pkosiec commented May 24, 2022

pkosiec commented May 25, 2022

PrasadG193 commented May 25, 2022

pkosiec commented May 25, 2022

PrasadG193 commented May 25, 2022

PrasadG193 left a comment

Update Go, Kubernetes and other dependencies #582

Update Go, Kubernetes and other dependencies #582

Conversation

pkosiec commented May 19, 2022 • edited Loading

ISSUE TYPE

SUMMARY

TESTING

pkosiec May 20, 2022

Choose a reason for hiding this comment

pkosiec May 20, 2022

Choose a reason for hiding this comment

pkosiec commented May 20, 2022 • edited Loading

pkosiec May 20, 2022

Choose a reason for hiding this comment

PrasadG193 commented May 21, 2022

pkosiec commented May 24, 2022

pkosiec commented May 25, 2022

PrasadG193 commented May 25, 2022

pkosiec commented May 25, 2022

PrasadG193 commented May 25, 2022

PrasadG193 left a comment

Choose a reason for hiding this comment

pkosiec commented May 19, 2022 •

edited

Loading

pkosiec commented May 20, 2022 •

edited

Loading