[ChaosCenter]: Migrating graphql-server from Gorilla mux to Gin (#3923) #3923

namkyu1999 · 2023-03-16T05:10:21Z

Proposed changes

Migrate from Gorilla Mux to Gin because of two main reasons.

Gorilla Mux is no longer actively maintained
Unifying the router framework used in backend applications (Authentication, GraphQL ...)

Issue No. #3919

Types of changes

What types of changes does your code introduce to Litmus? Put an x in the boxes that apply

New feature (non-breaking change which adds functionality)
Bugfix (non-breaking change which fixes an issue)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation Update (if none of the other choices applies)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

I have read the CONTRIBUTING doc
I have signed the commit for DCO to be passed.
Lint and unit tests pass locally with my changes
I have added tests that prove my fix is effective or that my feature works (if appropriate)
I have added necessary documentation (if appropriate)

Dependency

Please add the links to the dependent PR need to be merged before this (if any).

Special notes for your reviewer: @S-ayanide @amityt

Signed-off-by: Namkyu Park <lak9348@gmail.com> Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

litmus-portal/graphql-server/pkg/myhub/myhub.go

S-ayanide · 2023-03-17T07:39:00Z

There seems to be some linting and import order issues, can you run a gofmt in your change files!

Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

namkyu1999 · 2023-03-20T01:10:08Z

There seems to be some linting and import order issues, can you run a gofmt in your change files!

I fixed all @S-ayanide !

litmus-portal/graphql-server/pkg/myhub/myhub.go

…ux-to-gin Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

litmus-portal/graphql-server/pkg/chaoshub/chaoshub.go

Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

litmus-portal/graphql-server/pkg/chaoshub/chaoshub.go

Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

litmus-portal/graphql-server/pkg/chaoshub/chaoshub.go

litmus-portal/graphql-server/pkg/handlers/readiness_handler.go

imrajdas

Please fix requested codeql issues in your PR

litmus-portal/graphql-server/pkg/chaoshub/service.go

…schaos#3933) * fix: Move to using interface for better mocking at chaoshub Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr> * fix: delete operator interface and linting codes Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr> * fix: change db instance to local variable Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr> * fix: passing operator interface for better mocking Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr> --------- Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

…ux-to-gin Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

namkyu1999 · 2023-04-07T09:47:33Z

Please fix requested codeql issues in your PR

Hey @imrajdas,
CodeQL issues that I encountered is one that existed before my PR.
The problem is our backend code use variable given by user to os file open.
I think this task is independently from this pr. So If ok, can I raise another issue to handle it?

As a temporary measure, I wrote code to remove "../" , "..", etc. if they were added to the data by the API requestor.
replacer := strings.NewReplacer("../", "", "./", "", "/", "", "..", "")

imrajdas · 2023-04-07T12:12:33Z

As a temporary measure, I wrote code to remove "../" , "..", etc. if they were added to the data by the API requestor.

Ok, we can take it in other PR.

imrajdas · 2023-04-07T12:14:47Z

@amityt - please do a final review before the merge

imrajdas · 2023-04-07T12:16:03Z

litmus-portal/graphql-server/go.mod

@@ -6,11 +6,12 @@ require (
 	github.com/99designs/gqlgen v0.11.3
 	github.com/argoproj/argo-workflows/v3 v3.3.1
 	github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32
+	github.com/gin-contrib/cors v1.3.1
+	github.com/gin-gonic/gin v1.7.7


1.7.7 is released in 2021. Can we use the latest one? If no major code change is required

@imrajdas let's take it in a different PR.

1.9.0 is the latest version.
But higher than 1.7.7, We need to upgrade the version of Go 1.17.
So I just use that version like the authentication server in litmusChaos

okay, lets do it in other PR

namkyu1999 force-pushed the mux-to-gin branch 2 times, most recently from 0df3e8c to 44ca043 Compare March 16, 2023 07:38

Migrate from Gorilla mux to Gin

c9c08d3

Signed-off-by: Namkyu Park <lak9348@gmail.com> Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

namkyu1999 force-pushed the mux-to-gin branch from 44ca043 to c9c08d3 Compare March 16, 2023 07:39

github-advanced-security bot found potential problems Mar 16, 2023

View reviewed changes

litmus-portal/graphql-server/pkg/myhub/myhub.go Fixed Show fixed Hide fixed

litmus-portal/graphql-server/pkg/myhub/myhub.go Fixed Show fixed Hide fixed

S-ayanide assigned namkyu1999 Mar 16, 2023

S-ayanide added litmus-portal /refactor Refactoring litmus-portal LFX-MENTORSHIP Linux Foundation Mentor ship Issue READY-FOR-REVIEW labels Mar 16, 2023

S-ayanide mentioned this pull request Mar 16, 2023

Migrate graphql-server from Gorilla mux to Gin #3919

Closed

S-ayanide requested a review from amityt March 17, 2023 07:22

S-ayanide self-requested a review March 17, 2023 07:39

Linting & Ordering codes

d6e352d

Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

S-ayanide reviewed Mar 22, 2023

View reviewed changes

litmus-portal/graphql-server/pkg/myhub/myhub.go Outdated Show resolved Hide resolved

Merge branch 'master' of https://github.com/litmuschaos/litmus into m…

5369e4b

…ux-to-gin Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

namkyu1999 commented Mar 23, 2023

View reviewed changes

litmus-portal/graphql-server/pkg/chaoshub/chaoshub.go Outdated Show resolved Hide resolved

github-advanced-security bot found potential problems Mar 23, 2023

View reviewed changes

litmus-portal/graphql-server/pkg/chaoshub/chaoshub.go Fixed Show fixed Hide fixed

litmus-portal/graphql-server/pkg/chaoshub/chaoshub.go Fixed Show fixed Hide fixed

amityt approved these changes Mar 27, 2023

View reviewed changes

amityt requested a review from S-ayanide March 27, 2023 04:04

gdsoumya requested changes Mar 27, 2023

View reviewed changes

litmus-portal/graphql-server/pkg/chaoshub/chaoshub.go Outdated Show resolved Hide resolved

fix: remove duplicated code

26a1619

Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

namkyu1999 requested review from gdsoumya and removed request for S-ayanide March 27, 2023 07:43

github-advanced-security bot found potential problems Mar 27, 2023

View reviewed changes

litmus-portal/graphql-server/pkg/chaoshub/chaoshub.go Fixed Show fixed Hide fixed

litmus-portal/graphql-server/pkg/chaoshub/chaoshub.go Fixed Show fixed Hide fixed

fix: adopt go fmt

6c7738a

Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

imrajdas reviewed Mar 29, 2023

View reviewed changes

litmus-portal/graphql-server/pkg/chaoshub/chaoshub.go Outdated Show resolved Hide resolved

imrajdas reviewed Mar 29, 2023

View reviewed changes

litmus-portal/graphql-server/pkg/chaoshub/chaoshub.go Outdated Show resolved Hide resolved

imrajdas reviewed Mar 29, 2023

View reviewed changes

litmus-portal/graphql-server/pkg/handlers/readiness_handler.go Show resolved Hide resolved

imrajdas reviewed Apr 7, 2023

View reviewed changes

github-advanced-security bot found potential problems Apr 7, 2023

View reviewed changes

litmus-portal/graphql-server/pkg/chaoshub/service.go Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems Apr 7, 2023

View reviewed changes

litmus-portal/graphql-server/pkg/chaoshub/service.go Fixed Show fixed Hide fixed

namkyu1999 and others added 5 commits April 7, 2023 17:35

fix: status code to constant variable

3493605

Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

fix: add logs when encounterd error

9e9377e

Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

fix: rename pkg name

b5b0b28

Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

fix: rename pkg, graphql_server to rest_handlers

c20456d

Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

namkyu1999 force-pushed the mux-to-gin branch 5 times, most recently from 9145dad to 9b8b5cb Compare April 7, 2023 09:15

namkyu1999 added 2 commits April 7, 2023 18:27

fix: resolve codeql issue

464cef0

Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

fix: add replacer value

04842e5

Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

namkyu1999 force-pushed the mux-to-gin branch 2 times, most recently from 25890b4 to 04842e5 Compare April 7, 2023 09:32

Merge branch 'master' of https://github.com/litmuschaos/litmus into m…

001aaf9

…ux-to-gin Signed-off-by: namkyu1999 <lak9348@konkuk.ac.kr>

namkyu1999 requested a review from imrajdas April 7, 2023 10:33

imrajdas approved these changes Apr 7, 2023

View reviewed changes

imrajdas reviewed Apr 7, 2023

View reviewed changes

amityt approved these changes Apr 7, 2023

View reviewed changes

imrajdas changed the title ~~Migrate from Gorilla mux to Gin~~ [ChaosCenter]: Migrating graphql-server from Gorilla mux to Gin (#3923) Apr 7, 2023

imrajdas merged commit c6cda32 into litmuschaos:master Apr 7, 2023

This was referenced May 10, 2023

Bump github.com/gin-gonic/gin from 1.7.7 to 1.9.0 in /litmus-portal/graphql-server #3966

Closed

Bump github.com/gin-gonic/gin from 1.7.7 to 1.9.0 in /litmus-portal/authentication #3965

Closed

namkyu1999 mentioned this pull request May 18, 2023

bump the Go image to 1.20 #3974

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ChaosCenter]: Migrating graphql-server from Gorilla mux to Gin (#3923) #3923

[ChaosCenter]: Migrating graphql-server from Gorilla mux to Gin (#3923) #3923

namkyu1999 commented Mar 16, 2023

S-ayanide commented Mar 17, 2023

namkyu1999 commented Mar 20, 2023

imrajdas left a comment

namkyu1999 commented Apr 7, 2023 •

edited

Loading

imrajdas commented Apr 7, 2023

imrajdas commented Apr 7, 2023

imrajdas Apr 7, 2023 •

edited

Loading

amityt Apr 7, 2023

namkyu1999 Apr 7, 2023

imrajdas Apr 7, 2023

[ChaosCenter]: Migrating graphql-server from Gorilla mux to Gin (#3923) #3923

[ChaosCenter]: Migrating graphql-server from Gorilla mux to Gin (#3923) #3923

Conversation

namkyu1999 commented Mar 16, 2023

Proposed changes

Types of changes

Checklist

Dependency

Special notes for your reviewer: @S-ayanide @amityt

S-ayanide commented Mar 17, 2023

namkyu1999 commented Mar 20, 2023

imrajdas left a comment

Choose a reason for hiding this comment

namkyu1999 commented Apr 7, 2023 • edited Loading

imrajdas commented Apr 7, 2023

imrajdas commented Apr 7, 2023

imrajdas Apr 7, 2023 • edited Loading

Choose a reason for hiding this comment

amityt Apr 7, 2023

Choose a reason for hiding this comment

namkyu1999 Apr 7, 2023

Choose a reason for hiding this comment

imrajdas Apr 7, 2023

Choose a reason for hiding this comment

namkyu1999 commented Apr 7, 2023 •

edited

Loading

imrajdas Apr 7, 2023 •

edited

Loading