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.

Sign up for GitHub

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

feat: add etcd basic auth support #951

Merged
merged 23 commits into from
Dec 8, 2020
+98 −16
Merged

feat: add etcd basic auth support #951

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
dea6510
add etcd basic auth support
Nov 30, 2020
dc781df
Merge branch 'upstream' into etcd-basic-auth
Dec 1, 2020
b9a493c
Merge branch 'upstream' into etcd-basic-auth
Dec 2, 2020
a6429c6
etcd-basic-auth modification test
Dec 2, 2020
e31164b
not enable etcd auth for default
Dec 2, 2020
19f463d
Merge branch 'master' into etcd-basic-auth
Dec 2, 2020
c6c6297
Merge branch 'upstream' into etcd-basic-auth
Dec 2, 2020
c029e0d
Merge branch 'etcd-basic-auth' of github.com:JieTrancender/apisix-das…
Dec 2, 2020
3274519
Merge branch 'upstream' into etcd-basic-auth
Dec 2, 2020
9ae2397
Merge branch 'upstream' into etcd-basic-auth
Dec 3, 2020
8054f30
add etcd auth test
Dec 4, 2020
21248fe
add more etcd basic auth test
Dec 4, 2020
780abdf
fix ci_test wrong
Dec 4, 2020
4f94e84
fix cli_test wrong
Dec 4, 2020
d550545
debug cli_test wrong
Dec 4, 2020
4c3bc5c
fix cli_test wrong
Dec 4, 2020
6dc2fad
delete debug msg and resolve this test_cli wrong
Dec 4, 2020
40197e7
adjust package import order
Dec 4, 2020
ea72f57
more validation test
Dec 5, 2020
f40b7f7
make something more semantic
Dec 5, 2020
ceb09ea
Merge branch 'upstream' into etcd-basic-auth
Dec 7, 2020
2528a86
remove -X when use -d flag
Dec 7, 2020
df01aaa
adjust conf format
Dec 7, 2020
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
22 changes: 19 additions & 3 deletions api/conf/conf.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ var (
WorkDir = "."
ServerHost = "127.0.0.1"
ServerPort = 80
ETCDEndpoints = []string{"127.0.0.1:2379"}
ETCDConfig *Etcd
ErrorLogLevel = "warn"
ErrorLogPath = "logs/error.log"
UserList = make(map[string]User, 2)
Expand All @@ -55,6 +55,8 @@ var (

type Etcd struct {
Endpoints []string
Username string
Password string
}

type Listen struct {
Expand Down Expand Up @@ -128,9 +130,9 @@ func setConf() {
ServerHost = config.Conf.Listen.Host
}

//etcd
// for etcd
if len(config.Conf.Etcd.Endpoints) > 0 {
ETCDEndpoints = config.Conf.Etcd.Endpoints
initEtcdConfig(config.Conf.Etcd)
}

//error log
Expand Down Expand Up @@ -180,3 +182,17 @@ func initSchema() {
Schema = gjson.ParseBytes(schemaContent)
}
}

// initialize etcd config
func initEtcdConfig(conf Etcd) {
var endpoints = []string{"127.0.0.1:2379"}
if len(conf.Endpoints) > 0 {
endpoints = conf.Endpoints
}

ETCDConfig = &Etcd{
Endpoints: endpoints,
Username: conf.Username,
Password: conf.Password,
}
}
4 changes: 4 additions & 0 deletions api/conf/conf.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ conf:
etcd:
endpoints: # supports defining multiple etcd host addresses for an etcd cluster
- 127.0.0.1:2379

# etcd basic auth info
# username: "root" # ignore this argument if not enable auth
# password: "123456" # ignore this argument if not enable auth
log:
error_log:
level: warn # supports levels, lower to higher: debug, info, warn, error, panic, fatal
Expand Down
1 change: 1 addition & 0 deletions api/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ github.com/shiningrush/droplet v0.2.1 h1:p2utttTbCfgiL+x0Zrb2jFeWspB7/o+v3e+R94G
github.com/shiningrush/droplet v0.2.1/go.mod h1:akW2vIeamvMD6zj6wIBfzYn6StGXBxwlW3gA+hcHu5M=
github.com/shiningrush/droplet v0.2.2 h1:jEqSGoJXlybt1yQdauu+waE+l7KYlguNs8VayMfQ96Q=
github.com/shiningrush/droplet v0.2.2/go.mod h1:akW2vIeamvMD6zj6wIBfzYn6StGXBxwlW3gA+hcHu5M=
github.com/shiningrush/droplet v0.2.3 h1:bzPDzkE0F54r94XsultGS8uAPeL3pZIRmjqM0zIlpeI=
github.com/shiningrush/droplet v0.2.3/go.mod h1:akW2vIeamvMD6zj6wIBfzYn6StGXBxwlW3gA+hcHu5M=
github.com/shiningrush/droplet/wrapper/gin v0.2.0 h1:LHkU+TbSkpePgXrTg3hJoSZlCMS03GeWMl0t+oLkd44=
github.com/shiningrush/droplet/wrapper/gin v0.2.0/go.mod h1:ZJu+sCRrVXn5Pg618c1KK3Ob2UiXGuPM1ROx5uMM9YQ=
Expand Down
7 changes: 5 additions & 2 deletions api/internal/core/storage/etcd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"time"

"github.com/apisix/manager-api/conf"
"github.com/apisix/manager-api/internal/utils"
"go.etcd.io/etcd/clientv3"
)
Expand All @@ -32,10 +33,12 @@ var (
type EtcdV3Storage struct {
}

func InitETCDClient(endpoints []string) error {
func InitETCDClient(etcdConf *conf.Etcd) error {
cli, err := clientv3.New(clientv3.Config{
Endpoints: endpoints,
Endpoints: etcdConf.Endpoints,
DialTimeout: 5 * time.Second,
Username: etcdConf.Username,
Password: etcdConf.Password,
})
if err != nil {
return fmt.Errorf("init etcd failed: %w", err)
Expand Down
3 changes: 2 additions & 1 deletion api/internal/handler/consumer/consumer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package consumer

import (
"encoding/json"
"github.com/apisix/manager-api/conf"
Copy link
Member

Choose a reason for hiding this comment

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

Should be under the standard library.

"testing"
"time"

Expand All @@ -32,7 +33,7 @@ import (

func TestConsumer(t *testing.T) {
// init
err := storage.InitETCDClient([]string{"127.0.0.1:2379"})
err := storage.InitETCDClient(conf.ETCDConfig)
assert.Nil(t, err)
err = store.InitStores()
assert.Nil(t, err)
Expand Down
5 changes: 3 additions & 2 deletions api/internal/handler/route/route_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,15 @@ import (
"github.com/shiningrush/droplet/data"
"github.com/stretchr/testify/assert"

"github.com/apisix/manager-api/conf"
"github.com/apisix/manager-api/internal/core/entity"
"github.com/apisix/manager-api/internal/core/storage"
"github.com/apisix/manager-api/internal/core/store"
)

func TestRoute(t *testing.T) {
// init
err := storage.InitETCDClient([]string{"127.0.0.1:2379"})
err := storage.InitETCDClient(conf.ETCDConfig)
assert.Nil(t, err)
err = store.InitStores()
assert.Nil(t, err)
Expand Down Expand Up @@ -989,7 +990,7 @@ func TestRoute(t *testing.T) {

func Test_Route_With_Script(t *testing.T) {
// init
err := storage.InitETCDClient([]string{"127.0.0.1:2379"})
err := storage.InitETCDClient(conf.ETCDConfig)
assert.Nil(t, err)
err = store.InitStores()
assert.Nil(t, err)
Expand Down
3 changes: 2 additions & 1 deletion api/internal/handler/service/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,15 @@ import (
"github.com/shiningrush/droplet"
"github.com/stretchr/testify/assert"

"github.com/apisix/manager-api/conf"
"github.com/apisix/manager-api/internal/core/entity"
"github.com/apisix/manager-api/internal/core/storage"
"github.com/apisix/manager-api/internal/core/store"
)

func TestService(t *testing.T) {
// init
err := storage.InitETCDClient([]string{"127.0.0.1:2379"})
err := storage.InitETCDClient(conf.ETCDConfig)
assert.Nil(t, err)
err = store.InitStores()
assert.Nil(t, err)
Expand Down
3 changes: 2 additions & 1 deletion api/internal/handler/ssl/ssl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,15 @@ import (
"github.com/shiningrush/droplet"
"github.com/stretchr/testify/assert"

"github.com/apisix/manager-api/conf"
"github.com/apisix/manager-api/internal/core/entity"
"github.com/apisix/manager-api/internal/core/storage"
"github.com/apisix/manager-api/internal/core/store"
)

func TestSSL(t *testing.T) {
// init
err := storage.InitETCDClient([]string{"127.0.0.1:2379"})
err := storage.InitETCDClient(conf.ETCDConfig)
assert.Nil(t, err)
err = store.InitStores()
assert.Nil(t, err)
Expand Down
3 changes: 2 additions & 1 deletion api/internal/handler/upstream/upstream_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/shiningrush/droplet"
"github.com/stretchr/testify/assert"

"github.com/apisix/manager-api/conf"
"github.com/apisix/manager-api/internal/core/entity"
"github.com/apisix/manager-api/internal/core/storage"
"github.com/apisix/manager-api/internal/core/store"
Expand All @@ -34,7 +35,7 @@ var upstreamHandler *Handler

func TestUpstream(t *testing.T) {
// init
err := storage.InitETCDClient([]string{"127.0.0.1:2379"})
err := storage.InitETCDClient(conf.ETCDConfig)
assert.Nil(t, err)
err = store.InitStores()
assert.Nil(t, err)
Expand Down
11 changes: 6 additions & 5 deletions api/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@ package main
import (
"context"
"fmt"
"github.com/apisix/manager-api/internal/handler"
"github.com/shiningrush/droplet"
"net/http"
"os"
"os/signal"
"syscall"
"time"

"github.com/apisix/manager-api/internal/handler"
"github.com/shiningrush/droplet"

"github.com/apisix/manager-api/conf"
"github.com/apisix/manager-api/internal"
"github.com/apisix/manager-api/internal/core/storage"
Expand All @@ -44,12 +45,12 @@ func main() {
newMws = append(newMws, mws[1:]...)
return newMws
}
if err := storage.InitETCDClient(conf.ETCDEndpoints); err != nil {
log.Error("init etcd client fail: %w", err)
if err := storage.InitETCDClient(conf.ETCDConfig); err != nil {
log.Errorf("init etcd client fail: %w", err)
panic(err)
}
if err := store.InitStores(); err != nil {
log.Error("init stores fail: %w", err)
log.Errorf("init stores fail: %w", err)
panic(err)
}
// routes
Expand Down
45 changes: 45 additions & 0 deletions api/test/shell/cli_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,48 @@ if [[ `grep -c "INFO" ./error.log` -eq '0' ]]; then
echo "failed: failed to write log on right level"
exit 1
fi


# etcd basic auth
# add root user
curl -L http://localhost:2379/v3/auth/user/add \
-X POST -d '{"name": "root", "password": "apisix-dashboard"}'
juzhiyuan marked this conversation as resolved.
Show resolved Hide resolved

# add root role
curl -L http://localhost:2379/v3/auth/role/add \
-X POST -d '{"name": "root"}'

# grant root role to root user
curl -L http://localhost:2379/v3/auth/user/grant \
-X POST -d '{"user": "root", "role": "root"}'

# enable auth
curl -L http://localhost:2379/v3/auth/enable -X POST -d '{}'

Copy link
Member

Choose a reason for hiding this comment

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

I think we could try to run ./manager-api that should fail before modify etcd config.

./manager-api &
Copy link
Member

Choose a reason for hiding this comment

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

how about ./manager-api > output.log 2>&1 here ?

sleep 3
pkill -f manager-api

# make sure it's wrong
if [[ `grep -c "etcdserver: user name is empty" ./error.log` -eq '0' ]]; then
echo "failed: failed to validate etcd basic auth"
exit 1
fi

# modify etcd auth config
sed -i '1,$s/# username: "root" # ignore this argument if not enable auth/username: "root"/g' conf/conf.yaml
sed -i '1,$s/# password: "123456" # ignore this argument if not enable auth/password: "apisix-dashboard"/g' conf/conf.yaml

./manager-api &

# validate process is right by requesting login api
resp=$(curl http://127.0.0.1:9000/apisix/admin/user/login -X POST -d '{"username":"admin", "password": "admin"}')
Copy link
Contributor

Choose a reason for hiding this comment

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

This API does not access store, we would better add a case to access store such as try create a consumer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, this api does't access store. but it's enough for verifing this test, if not this process has been killed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if do other things like create a consumer, clear dirty is necessary, making easy thing gets complex.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the discussion here is based on our goals:

  • If we only need to confirm that the process is alive, then accessing the /ping is a better choice, the /login has nothing to do with the modification this time
  • If we want to verify that the program can access ETCD actually, we should access an interface related to it. Because now e2e test is run with docker-compose, so I think not to clean dirty is fine, if you ensure consumer id is not same with other test cases.

I have not encountered a situation where the manager-api alive but cannot access the ETCD normally, so I think it is acceptable to just verify the process survival (although the latter would be better)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ye, you are right, i have done it.

token=$(echo "${resp}" | sed 's/{/\n/g' | sed 's/,/\n/g' | grep "token" | sed 's/:/\n/g' | sed '1d' | sed 's/}//g' | sed 's/"//g')
if [ -z "${token}" ]; then
echo "login failed"
fi

sleep 3
Copy link
Member

Choose a reason for hiding this comment

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

we could try some curl here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's right, i have a lazy just now.

pkill -f manager-api

check_logfile