Merge pull request #2065 from hashicorp/irindos-session-nullfkey

bug(session): null fkey trigger also checks for session terminated state
pull/2075/head
Irena Rindos 4 years ago committed by GitHub
commit a11b0e81a8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -9,6 +9,8 @@ Canonical reference for changes, improvements, and bugfixes for Boundary.
* controller: Do not shut down cluster listener when it receives an invalid
packet ([Issue](https://github.com/hashicorp/boundary/issues/2072),
[PR](https://github.com/hashicorp/boundary/pull/2073))
* session: update cancel_session() function to check for terminated state ([Issue](https://github.com/hashicorp/boundary/issues/2064),
[PR](https://github.com/hashicorp/boundary/pull/2065))
## 0.8.0 (2022/05/03)

@ -290,6 +290,7 @@ begin;
for each row execute procedure update_session_state_on_termination_reason();
-- Updated in 29/01_cancel_session_null_fkey
-- cancel_session will insert a cancel state for the session, if there's isn't
-- a canceled state already. It's used by cancel_session_with_null_fk.
create or replace function

@ -0,0 +1,37 @@
begin;
drop function cancel_session(in sessionId text);
-- Updates cancel_session() from 0/50_session to check if a session is either terminated or canceling
-- Sessions can progress directly to terminated without going through the canceling state
-- cancel_session will insert a cancel state for the session, if there's isn't
-- a canceled or terminated state already. It's used by cancel_session_with_null_fk.
create function
cancel_session(in sessionId text) returns void
as $$
declare
rows_affected numeric;
begin
insert into session_state(session_id, state)
select
sessionId::text, 'canceling'
from
session s
where
s.public_id = sessionId::text and
s.public_id not in (
select
session_id
from
session_state
where
session_id = sessionId::text and
state in('canceling','terminated')
) limit 1;
get diagnostics rows_affected = row_count;
if rows_affected > 1 then
raise exception 'cancel session: more than one row affected: %', rows_affected;
end if;
end;
$$ language plpgsql;
commit;

@ -0,0 +1,140 @@
package oss_test
import (
"context"
"testing"
"github.com/hashicorp/boundary/internal/authtoken"
"github.com/hashicorp/boundary/internal/db"
"github.com/hashicorp/boundary/internal/db/common"
"github.com/hashicorp/boundary/internal/db/schema"
"github.com/hashicorp/boundary/internal/host/static"
"github.com/hashicorp/boundary/internal/iam"
"github.com/hashicorp/boundary/internal/kms"
"github.com/hashicorp/boundary/internal/session"
"github.com/hashicorp/boundary/internal/target"
"github.com/hashicorp/boundary/internal/target/tcp"
"github.com/hashicorp/boundary/internal/types/resource"
"github.com/hashicorp/boundary/testing/dbtest"
"github.com/stretchr/testify/require"
)
func TestMigrations_SessionFKeyDelete(t *testing.T) {
t.Parallel()
require := require.New(t)
const (
priorMigration = 28002
currentMigration = 29001
)
dialect := dbtest.Postgres
ctx := context.Background()
c, u, _, err := dbtest.StartUsingTemplate(dialect, dbtest.WithTemplate(dbtest.Template1))
require.NoError(err)
t.Cleanup(func() {
require.NoError(c())
})
d, err := common.SqlOpen(dialect, u)
require.NoError(err)
// migration to the prior migration (before the one we want to test)
m, err := schema.NewManager(ctx, schema.Dialect(dialect), d, schema.WithEditions(
schema.TestCreatePartialEditions(schema.Dialect(dialect), schema.PartialEditions{"oss": priorMigration}),
))
require.NoError(err)
require.NoError(m.ApplyMigrations(ctx))
state, err := m.CurrentState(ctx)
require.NoError(err)
want := &schema.State{
Initialized: true,
Editions: []schema.EditionState{
{
Name: "oss",
BinarySchemaVersion: priorMigration,
DatabaseSchemaVersion: priorMigration,
DatabaseSchemaState: schema.Equal,
},
},
}
require.Equal(want, state)
// Seed the database with test data
dbType, err := db.StringToDbType(dialect)
require.NoError(err)
conn, err := db.Open(dbType, u)
require.NoError(err)
rw := db.New(conn)
wrapper := db.TestWrapper(t)
org, prj := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper))
require.NotNil(prj)
hc := static.TestCatalogs(t, conn, prj.GetPublicId(), 1)[0]
hs := static.TestSets(t, conn, hc.GetPublicId(), 1)[0]
h := static.TestHosts(t, conn, hc.GetPublicId(), 1)[0]
static.TestSetMembers(t, conn, hs.GetPublicId(), []*static.Host{h})
tar := tcp.TestTarget(ctx, t, conn, prj.GetPublicId(), "test", target.WithHostSources([]string{hs.GetPublicId()}))
kmsCache := kms.TestKms(t, conn, wrapper)
targetRepo, err := target.NewRepository(rw, rw, kmsCache)
require.NoError(err)
serverId := "worker"
tofu := session.TestTofu(t)
session.TestWorker(t, conn, wrapper, session.WithServerId(serverId))
at := authtoken.TestAuthToken(t, conn, kmsCache, org.GetPublicId())
uId := at.GetIamUserId()
sess := session.TestSession(t, conn, wrapper, session.ComposedOf{
UserId: uId,
HostId: h.GetPublicId(),
TargetId: tar.GetPublicId(),
HostSetId: hs.GetPublicId(),
AuthTokenId: at.GetPublicId(),
ScopeId: prj.GetPublicId(),
Endpoint: "tcp://127.0.0.1:22",
ConnectionLimit: 1,
})
sessionRepo, err := session.NewRepository(rw, rw, kmsCache)
require.NoError(err)
// Create and terminate session without canceling
_, _, err = sessionRepo.ActivateSession(ctx, sess.PublicId, sess.Version, serverId, resource.Worker.String(), tofu)
require.NoError(err)
session.TestState(t, conn, sess.PublicId, session.StatusTerminated)
// Delete target; expect a session_state violation and failure to delete target
rows, err := targetRepo.DeleteTarget(ctx, tar.GetPublicId())
require.Errorf(err, "target.(Repository).DeleteTarget: db.DoTx: target.(Repository).DeleteTarget: db.Delete: insert or update on table \"session_state\" violates foreign key constraint \"session_valid_state_enm_fkey\": integrity violation: error #1003")
require.Equal(0, rows)
// now we're ready for the migration we want to test.
m, err = schema.NewManager(ctx, schema.Dialect(dialect), d, schema.WithEditions(
schema.TestCreatePartialEditions(schema.Dialect(dialect), schema.PartialEditions{"oss": currentMigration}),
))
require.NoError(err)
require.NoError(m.ApplyMigrations(ctx))
state, err = m.CurrentState(ctx)
require.NoError(err)
want = &schema.State{
Initialized: true,
Editions: []schema.EditionState{
{
Name: "oss",
BinarySchemaVersion: currentMigration,
DatabaseSchemaVersion: currentMigration,
DatabaseSchemaState: schema.Equal,
},
},
}
require.Equal(want, state)
// Try to delete target again, should succeed without error
rows, err = targetRepo.DeleteTarget(ctx, tar.GetPublicId())
require.NoError(err)
require.Equal(1, rows)
}

@ -0,0 +1,25 @@
begin;
select plan(9);
-- Ensure session state table is populated
select is(count(*), 1::bigint) from session_state where session_id = 's1_____clare' and state='pending';
select is(count(*), 1::bigint) from session_state where session_id = 's1_____cindy' and state='terminated';
select is(count(*), 1::bigint) from session_state where session_id = 's1_____ciara' and state='canceling';
select is(count(*), 1::bigint) from session_state where session_id = 's1_____carly' and state='active';
-- Check that we have 4 sessions using this target
select is(count(*), 4::bigint) from session where target_id = 't_________cb';
-- Delete target, expect no errors
delete from target where public_id='t_________cb';
select is(count(*), 0::bigint) from target where public_id='t_________cb';
-- Ensure we no longer have sessions associated with this target
select is(count(*), 0::bigint) from session where target_id = 't_________cb';
-- Ensure sessions that were pending or active are now in canceling state
select is(count(*), 1::bigint) from session_state where state = 'canceling' and session_id = 's1_____clare';
select is(count(*), 1::bigint) from session_state where state = 'canceling' and session_id = 's1_____carly';
select * from finish();
rollback;

@ -1455,3 +1455,51 @@ func testSessionCredentialParams(t *testing.T, conn *db.DB, wrapper wrapping.Wra
params.DynamicCredentials = creds
return params
}
func TestRepository_deleteTargetFKey(t *testing.T) {
conn, _ := db.TestSetup(t, "postgres")
rw := db.New(conn)
wrapper := db.TestWrapper(t)
iamRepo := iam.TestRepo(t, conn, wrapper)
kms := kms.TestKms(t, conn, wrapper)
repo, err := NewRepository(rw, rw, kms)
targetRepo, err := target.NewRepository(rw, rw, kms)
require.NoError(t, err)
tests := []struct {
name string
state Status
}{
{
name: "Delete target for terminated session",
state: StatusTerminated,
},
{
name: "Delete target for canceling session",
state: StatusCanceling,
},
{
name: "Delete target for active session",
state: StatusActive,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert, require := assert.New(t), require.New(t)
c := TestSessionParams(t, conn, wrapper, iamRepo)
sesh := TestSession(t, conn, wrapper, c)
s := TestState(t, conn, sesh.PublicId, tt.state)
assert.Equal(tt.state, s.Status)
// Delete target associated with session; ensure target deletion with no state violations
rows, err := targetRepo.DeleteTarget(context.Background(), c.TargetId)
require.NoError(err)
assert.Equal(1, rows)
foundSession, _, err := repo.LookupSession(context.Background(), sesh.PublicId)
assert.NoError(err)
assert.Empty(foundSession.TargetId)
})
}
}

Loading…
Cancel
Save