Skip to content

Commit

Permalink
refactor(client): remove duplicate processing
Browse files Browse the repository at this point in the history
Signed-off-by: wafuwafu13 <mariobaske@i.softbank.jp>

refactor(clientv3): add comment

Signed-off-by: wafuwafu13 <mariobaske@i.softbank.jp>

refactor: use assert

Signed-off-by: wafuwafu13 <mariobaske@i.softbank.jp>

test: add TestIsUnavailableErr

Signed-off-by: wafuwafu13 <mariobaske@i.softbank.jp>

fix(client): change error message

Signed-off-by: wafuwafu13 <mariobaske@i.softbank.jp>

fix: change error message

Signed-off-by: wafuwafu13 <mariobaske@i.softbank.jp>

clientv3: add protection code to prevent SnapshotWithVersion from panicking

Signed-off-by: Benjamin Wang <wachao@vmware.com>

changelog: update the release date of etcd v3.4.23

Signed-off-by: Benjamin Wang <wachao@vmware.com>

remove the dependency on busybox

Signed-off-by: Benjamin Wang <wachao@vmware.com>

update changelog to update base image to static-debian11 and removd dependency on busybox

Signed-off-by: Benjamin Wang <wachao@vmware.com>

update nsswitch.conf file

Signed-off-by: Benjamin Wang <wachao@vmware.com>

tests: Reproduce issue 14685

Signed-off-by: Marek Siarkowicz <siarkowicz@google.com>

clientv3/naming/endpoints: fix endpoints prefix bug
fixes bug with multiple endpoints with same prefix

Signed-off-by: Ramil Mirhasanov <ramil600@yahoo.com>

build(deps): bump ossf/scorecard-action from 2.1.0 to 2.1.2

Bumps [ossf/scorecard-action](https://github.com/ossf/scorecard-action) from 2.1.0 to 2.1.2.
- [Release notes](https://github.com/ossf/scorecard-action/releases)
- [Changelog](https://github.com/ossf/scorecard-action/blob/main/RELEASE.md)
- [Commits](ossf/scorecard-action@937ffa9...e38b190)

---
updated-dependencies:
- dependency-name: ossf/scorecard-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Added 'secrets.GITHUB_TOKEN' for the static-analysis workflow

Refer to: arduino/setup-protoc#63

Signed-off-by: Benjamin Wang <wachao@vmware.com>
  • Loading branch information
wafuwafu13 committed Jan 5, 2023
1 parent dc680e3 commit b9c4635
Show file tree
Hide file tree
Showing 17 changed files with 173 additions and 77 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/scorecards.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
persist-credentials: false

- name: "Run analysis"
uses: ossf/scorecard-action@937ffa90d79c7d720498178154ad4c7ba1e4ad8c # tag=v2.1.0
uses: ossf/scorecard-action@e38b1902ae4f44df626f11ba0734b14fb91f8f86 # tag=v2.1.2
with:
results_file: results.sarif
results_format: sarif
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/static-analysis.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@ jobs:
uses: arduino/setup-protoc@64c0c85d18e984422218383b81c52f8b077404d3 # v1.1.2
with:
version: '3.14.0'
repo-token: ${{ secrets.GITHUB_TOKEN }}
- run: make verify
- run: make fix
8 changes: 7 additions & 1 deletion CHANGELOG/CHANGELOG-3.4.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,14 @@
Previous change logs can be found at [CHANGELOG-3.3](https://github.com/etcd-io/etcd/blob/main/CHANGELOG/CHANGELOG-3.3.md).

<hr>
## v3.4.24 (TBD)

## v3.4.23 (TBD)
### Other
- Updated [base image from base-debian11 to static-debian11 and removed dependency on busybox](https://github.com/etcd-io/etcd/pull/15038).

<hr>

## v3.4.23 (2022-12-21)

### Package `clientv3`
- Fix [Refreshing token on CommonName based authentication causes segmentation violation in client](https://github.com/etcd-io/etcd/pull/14792).
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG/CHANGELOG-3.5.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ Previous change logs can be found at [CHANGELOG-3.4](https://github.com/etcd-io/

### Security
- Use [distroless base image](https://github.com/etcd-io/etcd/pull/15016) to address critical Vulnerabilities.
- Updated [base image from base-debian11 to static-debian11 and removed dependency on busybox](https://github.com/etcd-io/etcd/pull/15037).
- Bumped [some dependencies](https://github.com/etcd-io/etcd/pull/15018) to address some HIGH Vulnerabilities.

### Go
Expand Down
14 changes: 5 additions & 9 deletions Dockerfile-release.amd64
Original file line number Diff line number Diff line change
@@ -1,20 +1,16 @@
FROM --platform=linux/amd64 busybox:1.34.1 as source
FROM --platform=linux/amd64 gcr.io/distroless/base-debian11

COPY --from=source /bin/sh /bin/sh
COPY --from=source /bin/mkdir /bin/mkdir
FROM --platform=linux/amd64 gcr.io/distroless/static-debian11

ADD etcd /usr/local/bin/
ADD etcdctl /usr/local/bin/
ADD etcdutl /usr/local/bin/
RUN mkdir -p /var/etcd/
RUN mkdir -p /var/lib/etcd/

WORKDIR /var/etcd/
WORKDIR /var/lib/etcd/

# Alpine Linux doesn't use pam, which means that there is no /etc/nsswitch.conf,
# but Golang relies on /etc/nsswitch.conf to check the order of DNS resolving
# (see https://github.com/golang/go/commit/9dee7771f561cf6aee081c0af6658cc81fac3918)
# To fix this we just create /etc/nsswitch.conf and add the following line:
RUN echo 'hosts: files mdns4_minimal [NOTFOUND=return] dns mdns4' >> /etc/nsswitch.conf
ADD nsswitch.conf /etc/nsswitch.conf

EXPOSE 2379 2380

Expand Down
12 changes: 4 additions & 8 deletions Dockerfile-release.arm64
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
FROM --platform=linux/arm64 busybox:1.34.1 as source
FROM --platform=linux/arm64 gcr.io/distroless/base-debian11

COPY --from=source /bin/sh /bin/sh
COPY --from=source /bin/mkdir /bin/mkdir
FROM --platform=linux/arm64 gcr.io/distroless/static-debian11

ADD etcd /usr/local/bin/
ADD etcdctl /usr/local/bin/
ADD etcdutl /usr/local/bin/
ADD var/etcd /var/etcd
ADD var/lib/etcd /var/lib/etcd
ENV ETCD_UNSUPPORTED_ARCH=arm64

WORKDIR /var/etcd/
WORKDIR /var/lib/etcd/

EXPOSE 2379 2380

Expand Down
11 changes: 4 additions & 7 deletions Dockerfile-release.ppc64le
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
FROM --platform=linux/ppc64le busybox:1.34.1 as source
FROM --platform=linux/ppc64le gcr.io/distroless/base-debian11

COPY --from=source /bin/sh /bin/sh
COPY --from=source /bin/mkdir /bin/mkdir
FROM --platform=linux/ppc64le gcr.io/distroless/static-debian11

ADD etcd /usr/local/bin/
ADD etcdctl /usr/local/bin/
ADD etcdutl /usr/local/bin/
ADD var/etcd /var/etcd
ADD var/lib/etcd /var/lib/etcd

WORKDIR /var/etcd/
WORKDIR /var/lib/etcd/

EXPOSE 2379 2380

Expand Down
12 changes: 4 additions & 8 deletions Dockerfile-release.s390x
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
FROM --platform=linux/s390x busybox:1.34.1 as source
FROM --platform=linux/s390x gcr.io/distroless/base-debian11

COPY --from=source /bin/sh /bin/sh
COPY --from=source /bin/mkdir /bin/mkdir

FROM --platform=linux/s390x gcr.io/distroless/static-debian11

ADD etcd /usr/local/bin/
ADD etcdctl /usr/local/bin/
ADD etcdutl /usr/local/bin/
ADD var/etcd /var/etcd
ADD var/lib/etcd /var/lib/etcd

WORKDIR /var/etcd/
WORKDIR /var/lib/etcd/

EXPOSE 2379 2380

Expand Down
73 changes: 58 additions & 15 deletions client/v3/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,15 @@ package clientv3
import (
"context"
"errors"
"fmt"
"io"
"net"
"sync"
"testing"
"time"

"github.com/stretchr/testify/assert"

"go.etcd.io/etcd/api/v3/etcdserverpb"
"go.etcd.io/etcd/api/v3/v3rpc/rpctypes"
"go.etcd.io/etcd/client/pkg/v3/testutil"
Expand Down Expand Up @@ -152,23 +155,63 @@ func TestDialNoTimeout(t *testing.T) {
}

func TestIsHaltErr(t *testing.T) {
if !isHaltErr(context.TODO(), errors.New("etcdserver: some etcdserver error")) {
t.Errorf(`error prefixed with "etcdserver: " should be Halted by default`)
}
if isHaltErr(context.TODO(), rpctypes.ErrGRPCStopped) {
t.Errorf("error %v should not halt", rpctypes.ErrGRPCStopped)
}
if isHaltErr(context.TODO(), rpctypes.ErrGRPCNoLeader) {
t.Errorf("error %v should not halt", rpctypes.ErrGRPCNoLeader)
}
assert.Equal(t,
isHaltErr(context.TODO(), errors.New("etcdserver: some etcdserver error")),
true,
"error created by errors.New should be unavailable error",
)
assert.Equal(t,
isHaltErr(context.TODO(), rpctypes.ErrGRPCStopped),
false,
fmt.Sprintf(`error "%v" should not be halt error`, rpctypes.ErrGRPCStopped),
)
assert.Equal(t,
isHaltErr(context.TODO(), rpctypes.ErrGRPCNoLeader),
false,
fmt.Sprintf(`error "%v" should not be halt error`, rpctypes.ErrGRPCNoLeader),
)
ctx, cancel := context.WithCancel(context.TODO())
if isHaltErr(ctx, nil) {
t.Errorf("no error and active context should not be Halted")
}
assert.Equal(t,
isHaltErr(ctx, nil),
false,
"no error and active context should be halt error",
)
cancel()
if !isHaltErr(ctx, nil) {
t.Errorf("cancel on context should be Halted")
}
assert.Equal(t,
isHaltErr(ctx, nil),
true,
"cancel on context should be halte error",
)
}

func TestIsUnavailableErr(t *testing.T) {
assert.Equal(t,
isUnavailableErr(context.TODO(), errors.New("etcdserver: some etcdserver error")),
false,
"error created by errors.New should not be unavailable error",
)
assert.Equal(t,
isUnavailableErr(context.TODO(), rpctypes.ErrGRPCStopped),
true,
fmt.Sprintf(`error "%v" should be unavailable error`, rpctypes.ErrGRPCStopped),
)
assert.Equal(t,
isUnavailableErr(context.TODO(), rpctypes.ErrGRPCNotCapable),
false,
fmt.Sprintf("error %v should not be unavailable error", rpctypes.ErrGRPCNotCapable),
)
ctx, cancel := context.WithCancel(context.TODO())
assert.Equal(t,
isUnavailableErr(ctx, nil),
false,
"no error and active context should not be unavailable error",
)
cancel()
assert.Equal(t,
isUnavailableErr(ctx, nil),
false,
"cancel on context should not be unavailable error",
)
}

func TestCloseCtxClient(t *testing.T) {
Expand Down
6 changes: 4 additions & 2 deletions client/v3/maintenance.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ func (m *maintenance) SnapshotWithVersion(ctx context.Context) (*SnapshotRespons
resp, err := ss.Recv()
if err != nil {
m.logAndCloseWithError(err, pw)
return nil, err
}
go func() {
// Saving response is blocking
Expand All @@ -260,10 +261,11 @@ func (m *maintenance) SnapshotWithVersion(ctx context.Context) (*SnapshotRespons
}
}
}()

return &SnapshotResponse{
Header: resp.Header,
Header: resp.GetHeader(),
Snapshot: &snapshotReadCloser{ctx: ctx, ReadCloser: pr},
Version: resp.Version,
Version: resp.GetVersion(),
}, err
}

Expand Down
9 changes: 6 additions & 3 deletions client/v3/naming/endpoints/endpoints_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ func (m *endpointManager) DeleteEndpoint(ctx context.Context, key string, opts .
}

func (m *endpointManager) NewWatchChannel(ctx context.Context) (WatchChannel, error) {
resp, err := m.client.Get(ctx, m.target, clientv3.WithPrefix(), clientv3.WithSerializable())
key := m.target + "/"
resp, err := m.client.Get(ctx, key, clientv3.WithPrefix(), clientv3.WithSerializable())
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -126,7 +127,8 @@ func (m *endpointManager) watch(ctx context.Context, rev int64, upch chan []*Upd

lg := m.client.GetLogger()
opts := []clientv3.OpOption{clientv3.WithRev(rev), clientv3.WithPrefix()}
wch := m.client.Watch(ctx, m.target, opts...)
key := m.target + "/"
wch := m.client.Watch(ctx, key, opts...)
for {
select {
case <-ctx.Done():
Expand Down Expand Up @@ -171,7 +173,8 @@ func (m *endpointManager) watch(ctx context.Context, rev int64, upch chan []*Upd
}

func (m *endpointManager) List(ctx context.Context) (Key2EndpointMap, error) {
resp, err := m.client.Get(ctx, m.target, clientv3.WithPrefix(), clientv3.WithSerializable())
key := m.target + "/"
resp, err := m.client.Get(ctx, key, clientv3.WithPrefix(), clientv3.WithSerializable())
if err != nil {
return nil, err
}
Expand Down
15 changes: 6 additions & 9 deletions client/v3/retry_interceptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,12 @@ func (c *Client) streamClientInterceptor(optFuncs ...retryOption) grpc.StreamCli
intOpts := reuseOrNewWithCallOptions(defaultOptions, optFuncs)
return func(ctx context.Context, desc *grpc.StreamDesc, cc *grpc.ClientConn, method string, streamer grpc.Streamer, opts ...grpc.CallOption) (grpc.ClientStream, error) {
ctx = withVersion(ctx)
// getToken automatically
// TODO(cfc4n): keep this code block, remove codes about getToken in client.go after pr #12165 merged.
if c.authTokenBundle != nil {
// equal to c.Username != "" && c.Password != ""
err := c.getToken(ctx)
if err != nil && rpctypes.Error(err) != rpctypes.ErrAuthNotEnabled {
c.GetLogger().Error("clientv3/retry_interceptor: getToken failed", zap.Error(err))
return nil, err
}
// getToken automatically. Otherwise, auth token may be invalid after watch reconnection because the token has expired
// (see https://github.com/etcd-io/etcd/issues/11954 for more).
err := c.getToken(ctx)
if err != nil {
c.GetLogger().Error("clientv3/retry_interceptor: getToken failed", zap.Error(err))
return nil, err
}
grpcOpts, retryOpts := filterCallOptions(opts)
callOpts := reuseOrNewWithCallOptions(intOpts, retryOpts)
Expand Down
15 changes: 1 addition & 14 deletions client/v3/snapshot/v3_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func SaveWithVersion(ctx context.Context, lg *zap.Logger, cfg clientv3.Config, d
start := time.Now()
resp, err := cli.SnapshotWithVersion(ctx)
if err != nil {
return resp.Version, err
return "", err
}
defer resp.Snapshot.Close()
lg.Info("fetching snapshot", zap.String("endpoint", cfg.Endpoints[0]))
Expand Down Expand Up @@ -99,16 +99,3 @@ func SaveWithVersion(ctx context.Context, lg *zap.Logger, cfg clientv3.Config, d
lg.Info("saved", zap.String("path", dbPath))
return resp.Version, nil
}

// Save fetches snapshot from remote etcd server and saves data
// to target path. If the context "ctx" is canceled or timed out,
// snapshot save stream will error out (e.g. context.Canceled,
// context.DeadlineExceeded). Make sure to specify only one endpoint
// in client configuration. Snapshot API must be requested to a
// selected node, and saved snapshot is the point-in-time state of
// the selected node.
// Deprecated: Use SaveWithVersion instead.
func Save(ctx context.Context, lg *zap.Logger, cfg clientv3.Config, dbPath string) error {
_, err := SaveWithVersion(ctx, lg, cfg, dbPath)
return err
}
22 changes: 22 additions & 0 deletions nsswitch.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# /etc/nsswitch.conf
#
# Example configuration of GNU Name Service Switch functionality.
# If you have the `glibc-doc-reference' and `info' packages installed, try:
# `info libc "Name Service Switch"' for information about this file.

passwd: compat
group: compat
shadow: compat
gshadow: files

hosts: files dns
networks: files

protocols: db files
services: db files
ethers: db files
rpc: db files

netgroup: nis
hosts: files mdns4_minimal [NOTFOUND=return] dns mdns4

2 changes: 2 additions & 0 deletions scripts/build-docker.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ mkdir -p "${IMAGEDIR}"/var/etcd
mkdir -p "${IMAGEDIR}"/var/lib/etcd
cp "${BINARYDIR}"/etcd "${BINARYDIR}"/etcdctl "${BINARYDIR}"/etcdutl "${IMAGEDIR}"

cp ./nsswitch.conf "${IMAGEDIR}"

cat ./"${DOCKERFILE}" > "${IMAGEDIR}"/Dockerfile

if [ -z "$TAG" ]; then
Expand Down
Loading

0 comments on commit b9c4635

Please sign in to comment.