Skip to content
This repository was archived by the owner on Jun 4, 2024. It is now read-only.

Replace slack and supporting libs with teleport dependencies#793

Merged
justinas merged 13 commits intomasterfrom
justinas/teleport-vendored-plugins
Apr 4, 2023
Merged

Replace slack and supporting libs with teleport dependencies#793
justinas merged 13 commits intomasterfrom
justinas/teleport-vendored-plugins

Conversation

@justinas
Copy link
Copy Markdown
Contributor

@justinas justinas commented Mar 23, 2023 •
edited
Loading

That code was vendored in teleport in gravitational/teleport#23045 , backport is pending in gravitational/teleport#23475 .

This makes the remaining code to depend on those packages in teleport.

  • Code removed: access/slack (but not access/slack/cmd/slack), access/common, lib
  • Import paths changed: access/*, event-handler, terraform

Please review stuff past the two first commits.

Drone test builds:
https://drone.platform.teleport.sh/gravitational/teleport-plugins/2363
https://drone.platform.teleport.sh/gravitational/teleport-plugins/2364
https://drone.platform.teleport.sh/gravitational/teleport-plugins/2365
https://drone.platform.teleport.sh/gravitational/teleport-plugins/2366
https://drone.platform.teleport.sh/gravitational/teleport-plugins/2367
https://drone.platform.teleport.sh/gravitational/teleport-plugins/2368
https://drone.platform.teleport.sh/gravitational/teleport-plugins/2369
https://drone.platform.teleport.sh/gravitational/teleport-plugins/2370
https://drone.platform.teleport.sh/gravitational/teleport-plugins/2371
https://drone.platform.teleport.sh/gravitational/teleport-plugins/2372

@justinas
Copy link
Copy Markdown
Contributor Author

justinas commented Mar 23, 2023 •
edited
Loading

Dependency review is failing due to a supposed CVE in teleport, however I think this is due to the fact that we do not use Go major version scheme with subfolders in teleport, and this makes Go.mod pseudo-version 1.3.3-0.20230323122326-68aaa58860d1. The lint thinks we're using teleport v1.3.3.

@justinas justinas force-pushed the justinas/teleport-vendored-plugins branch from f4c39f2 to 774c041 Compare March 23, 2023 14:42
@justinas justinas force-pushed the justinas/teleport-vendored-plugins branch from 2f5d0d8 to 3bd659e Compare March 23, 2023 15:43
@justinas justinas mentioned this pull request Mar 23, 2023
Merged
@justinas
Copy link
Copy Markdown
Contributor Author

justinas commented Mar 23, 2023 •
edited
Loading

Dependency review is failing due to a supposed CVE in teleport, however I think this is due to the fact that we do not use Go major version scheme with subfolders in teleport, and this makes Go.mod pseudo-version 1.3.3-0.20230323122326-68aaa58860d1. The lint thinks we're using teleport v1.3.3.

Made change in 3bd659e (per Mike's recommendation) to work around this, but we also need something like gravitational/shared-workflows#94 it seems.

Solved ✅

@justinas
Copy link
Copy Markdown
Contributor Author

justinas commented Mar 23, 2023

I'm opening this for initial review, keep in mind the TODOs in description.

@justinas justinas marked this pull request as ready for review March 23, 2023 16:06
r0mant
r0mant approved these changes Mar 25, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@r0mant r0mant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm once TODOs are addressed

@justinas justinas force-pushed the justinas/teleport-vendored-plugins branch from 04bd0c6 to b13a013 Compare March 29, 2023 12:56
@justinas
Copy link
Copy Markdown
Contributor Author

justinas commented Mar 29, 2023

lgtm once TODOs are addressed

@r0mant I've added an update script and bumped the dependencies to branch/v12 (since there is no v12 tag with these changes yet). PTAL.

@justinas justinas requested review from hugoShaka and r0mant March 29, 2023 13:41
r0mant
r0mant approved these changes Apr 3, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@r0mant r0mant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+243 -10,459 🥇

@justinas justinas enabled auto-merge (squash) April 4, 2023 10:37
@justinas justinas merged commit ad661e4 into master Apr 4, 2023
@justinas justinas mentioned this pull request Apr 4, 2023
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@klizhentas klizhentas Awaiting requested review from klizhentas klizhentas is a code owner

@russjones russjones Awaiting requested review from russjones russjones is a code owner

@Joerger Joerger Awaiting requested review from Joerger Joerger is a code owner

@tcsc tcsc Awaiting requested review from tcsc tcsc is a code owner

@zmb3 zmb3 Awaiting requested review from zmb3 zmb3 is a code owner

2 more reviewers

@r0mant r0mant r0mant approved these changes

@hugoShaka hugoShaka hugoShaka approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@justinas @r0mant @hugoShaka