Skip to content

Commit

Permalink
backport of commit 924d547
Browse files Browse the repository at this point in the history
  • Loading branch information
Dan-Heath authored Nov 14, 2024
1 parent 425f174 commit 59d8530
Show file tree
Hide file tree
Showing 54 changed files with 146 additions and 683 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ jobs:
repository: boundary
version: ${{ needs.set-product-version.outputs.product-version }}
product: ${{ env.PKG_NAME }}
- uses: actions/upload-artifact@50769540e7f4bd5e21e526ee35c689e35e0d6874 # v4.4.0
- uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a # v4.3.6
with:
name: metadata.json
path: ${{ steps.generate-metadata-file.outputs.filepath }}
Expand Down Expand Up @@ -279,12 +279,12 @@ jobs:
echo "RPM_PACKAGE=$(basename out/*.rpm)" >> "$GITHUB_ENV"
echo "DEB_PACKAGE=$(basename out/*.deb)" >> "$GITHUB_ENV"
- name: Upload RPM package
uses: actions/upload-artifact@50769540e7f4bd5e21e526ee35c689e35e0d6874 # v4.4.0
uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a # v4.3.6
with:
name: ${{ env.RPM_PACKAGE }}
path: out/${{ env.RPM_PACKAGE }}
- name: Upload DEB package
uses: actions/upload-artifact@50769540e7f4bd5e21e526ee35c689e35e0d6874 # v4.4.0
uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a # v4.3.6
with:
name: ${{ env.DEB_PACKAGE }}
path: out/${{ env.DEB_PACKAGE }}
Expand Down
12 changes: 6 additions & 6 deletions .github/workflows/enos-run.yml
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ jobs:
run: |
mv ${{ steps.download-docker.outputs.download-path }}/*.tar enos/support/boundary_docker_image.tar
- name: Set up Node.js
uses: actions/setup-node@0a44ba7841725637a19e28fa30b79a866c81b0a6 # v4.0.4
uses: actions/setup-node@1e60f620b9541d16bece96c5465dc8ee9832be0b # v4.0.3
if: contains(matrix.filter, 'e2e_ui')
with:
node-version: '16.x'
Expand Down Expand Up @@ -266,7 +266,7 @@ jobs:
SCENARIO=$(echo "${{ matrix.filter }}" | cut -d' ' -f1)
echo fragment="${SCENARIO}" >> "$GITHUB_OUTPUT"
- name: Upload e2e tests output
uses: actions/upload-artifact@50769540e7f4bd5e21e526ee35c689e35e0d6874 # v4.4.0
uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a # v4.3.6
with:
name: test-${{ steps.split.outputs.fragment }}
path: enos/*.log
Expand All @@ -279,7 +279,7 @@ jobs:
docker logs database
- name: Upload e2e UI tests debug info
if: contains(matrix.filter, 'e2e_ui') && steps.run.outcome == 'failure'
uses: actions/upload-artifact@50769540e7f4bd5e21e526ee35c689e35e0d6874 # v4.4.0
uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a # v4.3.6
with:
name: test-e2e-ui-debug
path: enos/support/src/boundary-ui/ui/admin/tests/e2e/artifacts/test-failures
Expand All @@ -292,7 +292,7 @@ jobs:
enos scenario launch --timeout 60m0s --chdir ./enos ${{ matrix.filter }}
- name: Upload Debug Data
if: ${{ always() && steps.run_retry.outcome == 'failure' }}
uses: actions/upload-artifact@50769540e7f4bd5e21e526ee35c689e35e0d6874 # v4.4.0
uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a # v4.3.6
with:
# The name of the artifact is the same as the matrix scenario name with the spaces replaced with underscores and colons replaced by equals.
name: ${{ steps.prepare_scenario.outputs.debug_data_artifact_name }}
Expand Down Expand Up @@ -327,7 +327,7 @@ jobs:
env
find ./enos -name "scenario.tf" -exec cat {} \;
- name: Send Slack message if Run and Retry fails (or if something else went wrong)
uses: slackapi/slack-github-action@37ebaef184d7626c5f204ab8d3baff4262dd30f0 # v1.27.0
uses: slackapi/slack-github-action@70cd7be8e40a46e8b0eced40b0de447bdb42f68e # v1.26.0
# steps.run.outcome reports as failure when there is an error in `Run Enos scenario`
# failure() captures errors before `Run Enos scenario`
# failure() does not capture errors in `Run Enos scenario` due to continue-on-error
Expand All @@ -341,7 +341,7 @@ jobs:
env:
SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOUNDARY_TEST_BOT_TOKEN }}
- name: Send Slack message if Run but Retry passes
uses: slackapi/slack-github-action@37ebaef184d7626c5f204ab8d3baff4262dd30f0 # v1.27.0
uses: slackapi/slack-github-action@70cd7be8e40a46e8b0eced40b0de447bdb42f68e # v1.26.0
if: ${{ steps.run.outcome == 'failure' && steps.run_retry.outcome != 'failure' }}
with:
channel-id: ${{ secrets.SLACK_BOUNDARY_TEST_BOT_CHANNEL_ID }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/fuzz.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ jobs:
run: go test ./internal/perms -fuzz=FuzzParse -fuzztime=30s
- name: Upload fuzz failure seed corpus as run artifact
if: failure()
uses: actions/upload-artifact@50769540e7f4bd5e21e526ee35c689e35e0d6874 # v4.4.0
uses: actions/upload-artifact@834a144ee995460fba8ed112a2fc961b36a5ec5a # v4.3.6
with:
name: fuzz-corpus
path: ./internal/perms/testdata/fuzz
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/security-scan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
cache: false

- name: Set up Python
uses: actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3 # v5.2.0
uses: actions/setup-python@39cd14951b08e74b54015e9e001cdefcf80e669f # v5.1.1
with:
python-version: 3.x

Expand Down Expand Up @@ -64,7 +64,7 @@ jobs:
python3 -m pip install semgrep==1.45.0
# CodeQL
LATEST=$(gh release list --repo https://github.com/github/codeql-action | cut -f 3 | grep codeql-bundle- | sort --version-sort | tail -n1)
LATEST=$(gh release list --repo https://github.com/github/codeql-action | cut -f 3 | sort --version-sort | tail -n1)
gh release download --repo https://github.com/github/codeql-action --pattern codeql-bundle-linux64.tar.gz "$LATEST"
tar xf codeql-bundle-linux64.tar.gz -C "$HOME/.bin"
Expand All @@ -79,7 +79,7 @@ jobs:
repository: "$PWD"

- name: Upload SARIF file
uses: github/codeql-action/upload-sarif@5618c9fc1e675841ca52c1c6b1304f5255a905a0 # codeql-bundle-v2.19.0
uses: github/codeql-action/upload-sarif@5c02493ebfd65b28fd3b082c65e5af2cd745d91f # codeql-bundle-v2.18.2
with:
sarif_file: results.sarif

4 changes: 2 additions & 2 deletions .github/workflows/test-cli-ui_oss.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ jobs:
path: /tmp/bats-cli-ui-deps
key: enos-bats-cli-ui-deps-jq-1.6-password-store-1.7.4-vault-1.12.2
- name: Set up Node for Bats install
uses: actions/setup-node@0a44ba7841725637a19e28fa30b79a866c81b0a6 # v4.0.4
uses: actions/setup-node@1e60f620b9541d16bece96c5465dc8ee9832be0b # v4.0.3
with:
node-version: 16
- name: Install Bats via NPM
Expand Down Expand Up @@ -112,7 +112,7 @@ jobs:
make -C internal/tests/cli test-vault-down
- name: Send Slack message
if: ${{ failure() }}
uses: slackapi/slack-github-action@37ebaef184d7626c5f204ab8d3baff4262dd30f0 # v1.27.0
uses: slackapi/slack-github-action@70cd7be8e40a46e8b0eced40b0de447bdb42f68e # v1.26.0
with:
channel-id: ${{ secrets.SLACK_BOUNDARY_TEST_BOT_CHANNEL_ID }}
payload: |
Expand Down
5 changes: 0 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,6 @@

Canonical reference for changes, improvements, and bugfixes for Boundary.

## Next

* Introduces soft-delete for users within the client cache.
([PR](https://github.com/hashicorp/boundary/pull/5173)).

## 0.18.0 (2024/10/01)
### New and Improved

Expand Down
1 change: 0 additions & 1 deletion enos/ci/service-user-iam/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ data "aws_iam_policy_document" "enos_policy_document" {
"ec2:RevokeSecurityGroupIngress",
"ec2:RunInstances",
"ec2:TerminateInstances",
"ec2:UnassignIpv6Addresses",
"elasticloadbalancing:AddTags",
"elasticloadbalancing:ApplySecurityGroupsToLoadBalancer",
"elasticloadbalancing:AttachLoadBalancerToSubnets",
Expand Down
8 changes: 4 additions & 4 deletions internal/clientcache/internal/cache/refresh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func testResolvableAliasStaticResourceRetrievalFunc(inFunc func(ctx context.Cont

// testNoRefreshRetrievalFunc simulates a controller that doesn't support refresh
// since it does not return any refresh token.
func testNoRefreshRetrievalFunc[T any](_ *testing.T) func(context.Context, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
func testNoRefreshRetrievalFunc[T any](t *testing.T) func(context.Context, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
return func(_ context.Context, _, _ string, _ RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
return nil, nil, "", ErrRefreshNotSupported
}
Expand All @@ -113,7 +113,7 @@ func testNoRefreshRetrievalFunc[T any](_ *testing.T) func(context.Context, strin
// testErroringForRefreshTokenRetrievalFunc returns a refresh token error when
// the refresh token is not empty. This is useful for testing behavior when
// the refresh token has expired or is otherwise invalid.
func testErroringForRefreshTokenRetrievalFunc[T any](_ *testing.T, ret []T) func(context.Context, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
func testErroringForRefreshTokenRetrievalFunc[T any](t *testing.T, ret []T) func(context.Context, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
return func(ctx context.Context, s1, s2 string, refToken RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
if refToken != "" {
return nil, nil, "", api.ErrInvalidListToken
Expand Down Expand Up @@ -158,7 +158,7 @@ func testStaticResourceRetrievalFuncForId[T any](t *testing.T, ret [][]T, remove
// since it does not return any refresh token. This is for retrieval
// functions that require an id be provided for listing purposes like when
// listing resolvable aliases.
func testNoRefreshRetrievalFuncForId[T any](_ *testing.T) func(context.Context, string, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
func testNoRefreshRetrievalFuncForId[T any](t *testing.T) func(context.Context, string, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
return func(_ context.Context, _, _, _ string, _ RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
return nil, nil, "", ErrRefreshNotSupported
}
Expand All @@ -169,7 +169,7 @@ func testNoRefreshRetrievalFuncForId[T any](_ *testing.T) func(context.Context,
// the refresh token has expired or is otherwise invalid. This is for retrieval
// functions that require an id be provided for listing purposes like when
// listing resolvable aliases.
func testErroringForRefreshTokenRetrievalFuncForId[T any](_ *testing.T, ret []T) func(context.Context, string, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
func testErroringForRefreshTokenRetrievalFuncForId[T any](t *testing.T, ret []T) func(context.Context, string, string, string, RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
return func(ctx context.Context, s1, s2, s3 string, refToken RefreshTokenValue) ([]T, []string, RefreshTokenValue, error) {
if refToken != "" {
return nil, nil, "", api.ErrInvalidListToken
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func TestLookupRefreshToken(t *testing.T) {
})

t.Run("unknown user", func(t *testing.T) {
got, err := r.lookupRefreshToken(ctx, &user{Id: "unknownUser", Address: "addr"}, targetResourceType)
got, err := r.lookupRefreshToken(ctx, &user{Id: "unkonwnUser", Address: "addr"}, targetResourceType)
assert.NoError(t, err)
assert.Empty(t, got)
})
Expand All @@ -209,11 +209,10 @@ func TestLookupRefreshToken(t *testing.T) {
require.NoError(t, r.rw.Create(ctx, known))

before := time.Now().Truncate(time.Millisecond).UTC()
_, err := r.rw.DoTx(ctx, 1, db.ExpBackoff{}, func(r db.Reader, w db.Writer) error {
r.rw.DoTx(ctx, 1, db.ExpBackoff{}, func(r db.Reader, w db.Writer) error {
require.NoError(t, upsertRefreshToken(ctx, w, known, targetResourceType, token))
return nil
})
require.NoError(t, err)

got, err := r.lookupRefreshToken(ctx, known, targetResourceType)
assert.NoError(t, err)
Expand Down
42 changes: 11 additions & 31 deletions internal/clientcache/internal/cache/repository_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ func upsertUserAndAuthToken(ctx context.Context, reader db.Reader, writer db.Wri
}

var users []*user
// we only want users that have not been soft deleted
if err := reader.SearchWhere(ctx, &users, "true", []any{}, db.WithLimit(-1), db.WithTable(activeUserTableName)); err != nil {
if err := reader.SearchWhere(ctx, &users, "true", []any{}, db.WithLimit(-1)); err != nil {
return errors.Wrap(ctx, err, op)
}
if len(users) <= usersLimit {
Expand Down Expand Up @@ -383,8 +382,6 @@ func cleanExpiredOrOrphanedAuthTokens(ctx context.Context, writer db.Writer, idT
return nil
}

const activeUserTableName = "user_active" // users that have not been soft deleted

// lookupUser returns a user if one is present in the repository or nil if not.
func (r *Repository) lookupUser(ctx context.Context, id string) (*user, error) {
const op = "cache.(Repository).lookupUser"
Expand All @@ -393,8 +390,7 @@ func (r *Repository) lookupUser(ctx context.Context, id string) (*user, error) {
return nil, errors.New(ctx, errors.InvalidParameter, op, "empty id")
}
ret := &user{Id: id}
// we only want users that have NOT been soft deleted
if err := r.rw.LookupById(ctx, ret, db.WithTable(activeUserTableName)); err != nil {
if err := r.rw.LookupById(ctx, ret); err != nil {
if errors.IsNotFoundError(err) {
return nil, nil
}
Expand All @@ -407,8 +403,7 @@ func (r *Repository) lookupUser(ctx context.Context, id string) (*user, error) {
func (r *Repository) listUsers(ctx context.Context) ([]*user, error) {
const op = "cache.(Repository).listUsers"
var ret []*user
// we only want users that have NOT been soft deleted
if err := r.rw.SearchWhere(ctx, &ret, "true", nil, db.WithTable(activeUserTableName)); err != nil {
if err := r.rw.SearchWhere(ctx, &ret, "true", nil); err != nil {
return nil, errors.Wrap(ctx, err, op)
}
return ret, nil
Expand Down Expand Up @@ -487,31 +482,16 @@ func deleteUser(ctx context.Context, w db.Writer, u *user) (int, error) {
case u.Id == "":
return db.NoRowsAffected, errors.New(ctx, errors.InvalidParameter, op, "missing id")
}
const (
// delete the user if they don't have any refresh tokens which are
// newer than 20 days (the refresh token expiration time)
deleteStmt = "delete from user where id = ? and id not in (select user_id from refresh_token where DATETIME('now', '-20 days') < datetime(create_time) )"

// fallback to soft deleting the user
softDeleteStmt = "update user set deleted_at = (strftime('%Y-%m-%d %H:%M:%f','now')) where id = ?"
)
// see if we should delete the user
rowsAffected, err := w.Exec(ctx, deleteStmt, []any{u.Id})
switch {
case err != nil:
return db.NoRowsAffected, errors.Wrap(ctx, err, op)
case rowsAffected > 0:
// if we deleted the user, we're done.
return rowsAffected, nil
}

// fallback to soft delete
rowsAffected, err = w.Exec(ctx, softDeleteStmt, []any{u.Id})
// TODO(https://github.com/go-gorm/gorm/issues/4879): Use the
// writer.Delete() function once the gorm bug is fixed. Until then
// the gorm driver for sqlite has an error which wont execute a
// delete correctly. as a work around we manually execute the
// query here.
n, err := w.Exec(ctx, "delete from user where id = ?", []any{u.Id})
if err != nil {
return db.NoRowsAffected, errors.Wrap(ctx, err, op)
err = errors.Wrap(ctx, err, op)
}

return rowsAffected, nil
return n, err
}

// user is a gorm model for the user table. It represents a user
Expand Down
Loading

0 comments on commit 59d8530

Please sign in to comment.