Skip to content

Commit

Permalink
feat: add migration script linter (apache#5648)
Browse files Browse the repository at this point in the history
* feat: add  migration script linter

* fix: migration script linting errors

* fix: linting

* fix: typos

* fix: package path
  • Loading branch information
klesh committed Jul 11, 2023
1 parent 5ad4696 commit 2be936a
Show file tree
Hide file tree
Showing 50 changed files with 921 additions and 88 deletions.
37 changes: 37 additions & 0 deletions .github/workflows/migration-script-lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#
# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

name: migration-script-lint
on:
push:
tags:
- v*
branches:
- main
pull_request:
jobs:
migration-script-lint:
name: migration-script-lint
runs-on: ubuntu-latest
container: mericodev/lake-builder:latest
steps:
- uses: actions/checkout@v3
- name: migration script linting
run: |
go version
cd backend
go run core/migration/linter/main.go -p backend $(find . -path "**/migrationscripts/**.go")
3 changes: 3 additions & 0 deletions backend/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -168,3 +168,6 @@ clean:

build-server-image:
docker build -t $(IMAGE_REPO)/devlake:$(TAG) --build-arg TAG=$(TAG) --build-arg SHA=$(SHA) --file ./Dockerfile .

migration-script-lint:
go run core/migration/linter/main.go -- $$(find . -path '**/migrationscripts/**.go')
205 changes: 205 additions & 0 deletions backend/core/migration/linter/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
/*
Licensed to the Apache Software Foundation (ASF) under one or more
contributor license agreements. See the NOTICE file distributed with
this work for additional information regarding copyright ownership.
The ASF licenses this file to You under the Apache License, Version 2.0
(the "License"); you may not use this file except in compliance with
the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package main

import (
"bufio"
"fmt"
"go/ast"
"go/parser"
"go/token"
"os"
"path"
"strconv"
"strings"
"text/template"

"github.com/spf13/cobra"
)

var moduleName = ""

func init() {
// prepare the module name
line := firstLineFromFile("go.mod")
moduleName = strings.Split(line, " ")[1]
}

func firstLineFromFile(path string) string {
inFile, err := os.Open(path)
if err != nil {
panic(err)
}
defer inFile.Close()

scanner := bufio.NewScanner(inFile)
for scanner.Scan() {
return scanner.Text()
}
panic(fmt.Errorf("empty file: " + path))
}

const (
LINT_ERROR = "error"
LINT_WARNING = "warning"
)

type LintMessage struct {
Level string
File string
Line int
Col int
EndCol int
Title string
Msg string
}

func lintMigrationScript(file string, allowedPkgs map[string]bool) []LintMessage {
msgs := make([]LintMessage, 0)
src, err := os.ReadFile(file)
if err != nil {
msgs = append(msgs, LintMessage{
Level: LINT_ERROR,
File: file,
Title: "Error reading file",
Msg: err.Error(),
})
return msgs
}
fset := token.NewFileSet()
f, err := parser.ParseFile(fset, file, src, 0)
if err != nil {
msgs = append(msgs, LintMessage{
Level: LINT_ERROR,
File: file,
Title: "Error parsing file",
Msg: err.Error(),
})
return msgs
}
// ast.Print(fset, f)
ast.Inspect(f, func(n ast.Node) bool {
switch x := n.(type) {
case *ast.ImportSpec:
importedPkgName, err := strconv.Unquote(x.Path.Value)
if err != nil {
panic(err)
}
// it is ok to use subpackages
filePkgName := path.Join(moduleName, path.Dir(file))
if strings.HasPrefix(importedPkgName, filePkgName) {
return true
}
// it is ok to use external libs, their behaviors are considered stable
if !strings.HasPrefix(importedPkgName, moduleName) {
return true
}
// it is ok if the package is whitelisted
if allowedPkgs[importedPkgName] {
return true
}
// we have a problem
// migration scripts are Immutable, meaning their behaviors should not be changed over time
// Relying on other packages may break the constraint and cause unexpected side-effects.
// You may add the package to the whitelist by the -a option if you are sure it is OK
pos := fset.Position(n.Pos())
msgs = append(msgs, LintMessage{
Level: LINT_WARNING,
File: file,
Title: "Package not allowed",
Msg: fmt.Sprintf("%s imports forbidden package %s", file, x.Path.Value),
Line: pos.Line,
Col: pos.Column,
EndCol: pos.Column + len(x.Path.Value),
})
}
return true
})
return msgs
}

func main() {
cmd := &cobra.Command{Use: "migration script linter"}
prefix := cmd.Flags().StringP("prefix", "p", "", "path prefix if your go.mod resides in a subfolder")
allowedPkg := cmd.Flags().StringArrayP(
"allowed-pkg",
"a",
[]string{
"github.com/apache/incubator-devlake/core/config",
"github.com/apache/incubator-devlake/core/context",
"github.com/apache/incubator-devlake/core/dal",
"github.com/apache/incubator-devlake/core/errors",
"github.com/apache/incubator-devlake/helpers/migrationhelper",
"github.com/apache/incubator-devlake/core/models/migrationscripts/archived",
"github.com/apache/incubator-devlake/core/plugin",
"github.com/apache/incubator-devlake/helpers/pluginhelper/api",
},
"package that allowed to be used in a migration script. e.g.: github.com/apache/incubator-devlake/core/context",
)

cmd.Run = func(cmd *cobra.Command, args []string) {
allowedPkgs := make(map[string]bool, len(*allowedPkg))
for _, p := range *allowedPkg {
allowedPkgs[p] = true
}
warningTpl, err := template.New("warning").Parse("::warning file={{ .File }},line={{ .Line }},col={{ .Col }},endColumn={{ .EndCol }}::{{ .Msg }}")
if err != nil {
panic(err)
}
errorTpl, err := template.New("error").Parse("::error file={{ .File }},line={{ .Line }},endLine={{ .Col }},title={{ .Title }}::{{ .Msg }}")
if err != nil {
panic(err)
}
localTpl, err := template.New("local").Parse("{{ .Level }}: {{ .Msg }}\n\t{{ .File }}:{{ .Line }}:{{ .Col }}")
if err != nil {
panic(err)
}
exitCode := 0
for _, file := range args {
msgs := lintMigrationScript(file, allowedPkgs)
if len(msgs) == 0 {
continue
}
for _, msg := range msgs {
var tpl *template.Template
if *prefix != "" {
// github actions need root relative path for annotation to show up in the PR
msg.File = path.Join(*prefix, file)
tpl = errorTpl
if msg.Level == LINT_WARNING {
tpl = warningTpl
}
} else {
// we are running locally in the `backend` folder, use another format to make fixing easier
tpl = localTpl
}
err = tpl.Execute(os.Stderr, msg)
if err != nil {
panic(err)
}
os.Stderr.WriteString("\n")
exitCode = 1
}
}
os.Exit(exitCode)
}
err := cmd.Execute()
if err != nil {
panic(err)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ package migrationscripts
import (
"github.com/apache/incubator-devlake/core/context"
"github.com/apache/incubator-devlake/core/errors"
"github.com/apache/incubator-devlake/core/models/domainlayer"
"github.com/apache/incubator-devlake/core/models/migrationscripts/archived"
"github.com/apache/incubator-devlake/core/plugin"
)

Expand All @@ -29,7 +29,7 @@ var _ plugin.MigrationScript = (*commitLineChange)(nil)
type commitLineChange struct{}

type commitLineChange20220918 struct {
domainlayer.DomainEntity
archived.DomainEntity
Id string `gorm:"type:varchar(255);primaryKey"`
CommitSha string `gorm:"type:varchar(40);"`
NewFilePath string `gorm:"type:varchar(255);"`
Expand Down
4 changes: 2 additions & 2 deletions backend/core/models/migrationscripts/20220927_add_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ package migrationscripts
import (
"github.com/apache/incubator-devlake/core/context"
"github.com/apache/incubator-devlake/core/errors"
"github.com/apache/incubator-devlake/core/models/common"
"github.com/apache/incubator-devlake/core/models/migrationscripts/archived"
"github.com/apache/incubator-devlake/core/plugin"
)

var _ plugin.MigrationScript = (*addRepoSnapshot)(nil)

type repoSnapshot20220918 struct {
common.NoPKModel
archived.NoPKModel
RepoId string `gorm:"primaryKey;type:varchar(255)"`
CommitSha string `gorm:"primaryKey;type:varchar(40);"`
FilePath string `gorm:"primaryKey;type:varchar(255);"`
Expand Down
2 changes: 1 addition & 1 deletion backend/plugins/jira/api/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func TestConnection(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput,
if resBody.DeploymentType == models.DeploymentServer {
// only support 8.x.x or higher
if versions := resBody.VersionNumbers; len(versions) == 3 && versions[0] < 7 {
return nil, errors.Default.New(fmt.Sprintf("%s Support JIRA Server 8+ only", serverInfoFail))
return nil, errors.Default.New(fmt.Sprintf("%s Support JIRA Server 7+ only", serverInfoFail))
}
}
if res.StatusCode != http.StatusOK {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@ limitations under the License.
package archived

import (
"github.com/apache/incubator-devlake/core/models/common"
"time"

"github.com/apache/incubator-devlake/core/models/migrationscripts/archived"
)

type Assignment struct {
common.NoPKModel
archived.NoPKModel
ConnectionId uint64
IncidentNumber int `gorm:"primaryKey"`
UserId string `gorm:"primaryKey"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@ limitations under the License.
package archived

import (
"github.com/apache/incubator-devlake/core/models/common"
"time"

"github.com/apache/incubator-devlake/core/models/migrationscripts/archived"
)

type Incident struct {
common.NoPKModel
archived.NoPKModel
ConnectionId uint64 `gorm:"primaryKey"`
Number int `gorm:"primaryKey"`
Url string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ limitations under the License.
package archived

import (
"github.com/apache/incubator-devlake/core/models/common"
"github.com/apache/incubator-devlake/core/models/migrationscripts/archived"
)

type (
Service struct {
common.NoPKModel
archived.NoPKModel
ConnectionId uint64 `gorm:"primaryKey"`
Id string `gorm:"primaryKey;autoIncremental:false"`
Url string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ limitations under the License.

package archived

import "github.com/apache/incubator-devlake/core/models/common"
import (
"github.com/apache/incubator-devlake/core/models/migrationscripts/archived"
)

type TransformationRules struct {
common.Model
archived.Model
Name string `gorm:"type:varchar(255);index:idx_name_github,unique"`
ConnectionId uint64
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@ limitations under the License.

package archived

import (
"github.com/apache/incubator-devlake/core/models/common"
)
import "github.com/apache/incubator-devlake/core/models/migrationscripts/archived"

type User struct {
common.NoPKModel
archived.NoPKModel
ConnectionId uint64 `gorm:"primaryKey"`
Id string `gorm:"primaryKey;autoIncremental:false"`
Url string
Expand Down
Loading

0 comments on commit 2be936a

Please sign in to comment.