Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extension compliance updates #35

Merged
merged 2 commits into from
Apr 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 45 additions & 13 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ on:
- v*
pull_request:

defaults:
run:
shell: bash

jobs:
build-with-xk6:
runs-on: ubuntu-latest
Expand All @@ -20,24 +24,52 @@ jobs:
- name: Install Go
uses: actions/setup-go@v3
with:
go-version: 1.17.x
- name: Retrieve get module name
run: |
echo "::set-output name=Module::$(go list -m)"
id: module-name
- name: Verify builds with xk6
go-version: 1.20.x
- name: Check build
run: |
go version
pwd && ls -l

go install go.k6.io/xk6/cmd/xk6@latest
xk6 build --with ${{ steps.module-name.outputs.Module }}=.
MODULE_NAME=$(go list -m)

xk6 build \
--output ./k6ext \
--with $MODULE_NAME="."
./k6ext version

run-unit-tests:
runs-on: ubuntu-latest
test-go-versions:
strategy:
fail-fast: false
matrix:
go-version: [1.19.x, 1.20.x, tip]
platform: [ubuntu-latest, windows-latest]
runs-on: ${{ matrix.platform }}
steps:
- name: Checkout code
uses: actions/checkout@v3
- name: Install Go
- name: Install Go ${{ matrix.go-version }}
if: matrix.go-version != 'tip'
uses: actions/setup-go@v3
with:
go-version: 1.17.x
- name: Run unit tests
run: go test -v -cover -race ./...
go-version: ${{ matrix.go-version }}
- name: Install Go stable
if: matrix.go-version == 'tip'
uses: actions/setup-go@v3
with:
go-version: 1.x
- name: Install Go tip
shell: bash
if: matrix.go-version == 'tip'
run: |
go install golang.org/dl/gotip@latest
gotip download
echo "GOROOT=$HOME/sdk/gotip" >> "$GITHUB_ENV"
echo "GOPATH=$HOME/go" >> "$GITHUB_ENV"
echo "$HOME/go/bin" >> "$GITHUB_PATH"
echo "$HOME/sdk/gotip/bin" >> "$GITHUB_PATH"
- name: Run tests
run: |
which go
go version
go test -race -timeout 60s ./...
41 changes: 21 additions & 20 deletions .github/workflows/linter.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,30 @@ jobs:
- name: Install Go
uses: actions/setup-go@v3
with:
go-version: 1.17.x
go-version: 1.20.x
- name: Check module dependencies
run: |
go version
test -z "$(go mod tidy && git status go.* --porcelain)"
go mod verify

# TODO: Enable linting and address linter errors, https://github.com/grafana/xk6-sql/issues/5
# run-golangci:
# runs-on: ubuntu-latest
# steps:
# - name: Checkout code
# uses: actions/checkout@v3
# - name: Install Go
# uses: actions/setup-go@v3
# with:
# go-version: 1.17.x
# - name: Retrieve golangci-lint version
# run: |
# echo "::set-output name=Version::$(head -n 1 "${GITHUB_WORKSPACE}/.golangci.yml" | tr -d '# ')"
# id: version
# - name: golangci-lint
# uses: golangci/golangci-lint-action@v3
# with:
# # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
# version: ${{ steps.version.outputs.Version }}
lint:
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v3
with:
fetch-depth: 0
- name: Install Go
uses: actions/setup-go@v3
with:
go-version: 1.20.x
- name: Retrieve golangci-lint version
run: |
echo "Version=$(head -n 1 "${GITHUB_WORKSPACE}/.golangci.yml" | tr -d '# ')" >> $GITHUB_OUTPUT
id: version
- name: golangci-lint
uses: golangci/golangci-lint-action@v3
with:
version: ${{ steps.version.outputs.Version }}
only-new-issues: true
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,5 @@ k6

.idea/
.DS_Store

*test.db
126 changes: 90 additions & 36 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# v1.43.0
# v1.52.2
# Please don't remove the first line. It is used in CI to determine the golangci version
run:
deadline: 5m
Expand All @@ -21,55 +21,109 @@ issues:
linters:
- cyclop
- dupl
- funlen
- gochecknoglobals
- gocognit
- funlen
- lll
- linters:
- paralleltest # false positive: https://github.com/kunwardeep/paralleltest/issues/8.
text: "does not use range value in test Run"
- linters:
- forbidigo
text: 'use of `os\.(SyscallError|Signal|Interrupt)` forbidden'

linters-settings:
nolintlint:
# Disable to ensure that nolint directives don't have a leading space. Default is true.
allow-leading-space: false
exhaustive:
default-signifies-exhaustive: true
govet:
check-shadowing: true
cyclop:
max-complexity: 25
maligned:
suggest-new: true
dupl:
threshold: 150
exhaustive:
default-signifies-exhaustive: true
funlen:
lines: 80
statements: 60
goconst:
min-len: 10
min-occurrences: 4
govet:
check-shadowing: true
maligned:
suggest-new: true
funlen:
lines: 80
statements: 60
forbidigo:
forbid:
- '^(fmt\\.Print(|f|ln)|print|println)$'
# Forbid everything in os, except os.Signal and os.SyscalError
- '^os\.(.*)$(# Using anything except Signal and SyscallError from the os package is forbidden )?'
# Forbid everything in syscall except the uppercase constants
- '^syscall\.[^A-Z_]+$(# Using anything except constants from the syscall package is forbidden )?'
- '^logrus\.Logger$'

linters:
enable-all: true
disable:
- exhaustivestruct
- gci
- gochecknoinits
- gocyclo # replaced by cyclop since it also calculates the package complexity
- godot
- godox
- goerr113 # most of the errors here are meant for humans
- goheader
- golint # this linter is deprecated
- gomnd
- gomodguard
- interfacer # deprecated
- ireturn
- maligned # replaced by govet 'fieldalignment'
- nlreturn
- scopelint # deprecated, replaced by exportloopref
- tagliatelle
- testpackage
- thelper
- varnamelen # disabled before the final decision in (https://github.com/grafana/k6/pull/2323)
- wrapcheck # a little bit too much for k6, maybe after https://github.com/tomarrell/wrapcheck/issues/2 is fixed
- wsl
disable-all: true
enable:
- asasalint
- asciicheck
- bidichk
- bodyclose
- contextcheck
- cyclop
- depguard
- dogsled
- dupl
- durationcheck
- errcheck
- errchkjson
- errname
- errorlint
- exhaustive
- exportloopref
- forbidigo
- forcetypeassert
- funlen
- gocheckcompilerdirectives
- gochecknoglobals
- gocognit
- goconst
- gocritic
- gofmt
- gofumpt
- goimports
- gomoddirectives
- goprintffuncname
- gosec
- gosimple
- govet
- importas
- ineffassign
- lll
- makezero
- misspell
- nakedret
- nestif
- nilerr
- nilnil
- noctx
- nolintlint
- nosprintfhostport
- paralleltest
- prealloc
- predeclared
- promlinter
- revive
- reassign
- rowserrcheck
- sqlclosecheck
- staticcheck
- stylecheck
- tenv
- tparallel
- typecheck
- unconvert
- unparam
- unused
- usestdlibvars
- wastedassign
- whitespace
fast: false
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Multi-stage build to generate custom k6 with extension
Copy link

Choose a reason for hiding this comment

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

I think we should drop this Dockerfile now that xk6 has one. And update the note in the README with the right command to build xk6-sql with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

xk6 image does not yet support CGO_ENABLED as required for SQLite3 support. I created an issue in xk6 for this.

Copy link

Choose a reason for hiding this comment

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

As I mentioned in the issue, this doesn't require any changes in the grafana/xk6 image.

So I'd still prefer removing this Dockerfile and the docker-run.sh script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should keep this until xk6 docker image is actually able to build the binary. Once we can build using CGO_ENABLED, I'll be more than happy to update the documentation and remove the Dockerfile and docker-run.sh files.

FROM golang:1.18-alpine as builder
FROM golang:1.20-alpine as builder
WORKDIR $GOPATH/src/go.k6.io/k6
ADD . .
RUN apk --no-cache add build-base git
Expand All @@ -9,7 +9,7 @@ RUN CGO_ENABLED=1 xk6 build \
--output /tmp/k6

# Create image for running your customized k6
FROM alpine:3.15
FROM alpine:3.17
RUN apk add --no-cache ca-certificates \
&& adduser -D -u 12345 -g 12345 k6
COPY --from=builder /tmp/k6 /usr/bin/k6
Expand Down
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ help:
## clean: Removes any previously created build artifacts.
clean:
rm -f ./k6
rm -f ./intg_test.db

## build: Builds a custom 'k6' with the local extension.
build:
Expand Down
12 changes: 6 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
This is a [k6](https://github.com/grafana/k6) extension using the
[xk6](https://github.com/grafana/xk6) system.

Supported RDBMSs: `mysql`, `postgres`, `sqlite3`, `sqlserver`. See the [tests](tests)
directory for examples. Other RDBMSs are not supported, see
Supported RDBMSs: `mysql`, `postgres`, `sqlite3`, `sqlserver`. See the [examples](examples)
directory for usage. Other RDBMSs are not supported, see
[details below](#support-for-other-rdbmss).


Expand Down Expand Up @@ -51,7 +51,7 @@ make
```
Once built, you can run your newly extended `k6` using:
```shell
./k6 run tests/sqlite3_test.js
./k6 run examples/sqlite3_test.js
```

## Example
Expand Down Expand Up @@ -147,12 +147,12 @@ built from the local source files.
```shell
docker build -t grafana/k6-for-sql:latest .
```
Using this image, you may then execute the [tests/sqlite3_test.js](tests/sqlite3_test.js) script
Using this image, you may then execute the [examples/sqlite3_test.js](examples/sqlite3_test.js) script
by running the following command:
```shell
docker run -v $PWD:/scripts -it --rm grafana/k6-for-sql:latest run /scripts/tests/sqlite3_test.js
docker run -v $PWD:/scripts -it --rm grafana/k6-for-sql:latest run /scripts/examples/sqlite3_test.js
```
For those on Mac or Linux, the `docker-run.sh` script simplifies the command:
```shell
./docker-run.sh tests/sqlite3_test.js
./docker-run.sh examples/sqlite3_test.js
```
6 changes: 3 additions & 3 deletions docker-run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@

Copy link

Choose a reason for hiding this comment

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

Same with this script, I would remove it. The way to simplify the Docker CLI is to use Docker Compose, but for this simple extension, I don't think we need it either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Leaving this as the CGO support is required to switch to the xk6 docker image.

set -e

if [ $# -ne 1 ]; then
echo "Usage: ./docker-run.sh <SCRIPT_NAME>"
if [ $# -lt 1 ]; then
echo "Usage: ./docker-run.sh <SCRIPT_NAME> [additional k6 args]"
exit 1
fi

# By default, we're assuming you created the extended k6 image as "grafana/k6-for-sql:latest".
# If not, override the name on the command-line with `IMAGE_NAME=...`.
IMAGE_NAME=${IMAGE_NAME:="grafana/k6-for-sql:latest"}

docker run -v $PWD:/scripts -it --rm $IMAGE_NAME run /scripts/$1
docker run -v $PWD:/scripts -it --rm $IMAGE_NAME run /scripts/$1 ${@:2}
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
Loading