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

DAT-14267: add liquibase-github-action to update mssql database #534

Merged
merged 20 commits into from
Apr 28, 2023

Conversation

jandroav
Copy link
Contributor

@jandroav jandroav commented Apr 11, 2023

feat(mssql.sql): add liquibase changeset to add lbuser and grant db_owner role to it
DAT-14267

feat(mssql.sql): add liquibase changeset to add lbuser and grant db_owner role to it
@jandroav
Copy link
Contributor Author

I am getting:

Unexpected error running Liquibase: Cannot invoke "Object.getClass()" because "unexpectedObject" is null

here: https://github.com/liquibase/liquibase-test-harness/actions/runs/4666624792/jobs/8261428473

It is failing in the drop-all step, before the new code. Any idea about what is happening? @yodzhubeiskyi Could you please take a look when you have time?

@jnewton03
Copy link
Contributor

@jandroav we should try to move away from the all-in-one Github action since it's deprecated

https://github.com/liquibase-github-actions/update is what we want to use instead

jandroav added 2 commits April 12, 2023 08:12
fix(gcp.yml): change 'changeLogFile' to 'changelogFile' in liquibase-github-action configuration
@jandroav
Copy link
Contributor Author

jandroav commented Apr 12, 2023

@jnewton03 Changed and run a build with logLevel: INFO. It seems liquibase can perform the drop but it fails at the end: https://github.com/liquibase/liquibase-test-harness/actions/runs/4675293365/jobs/8280278955

Also added failOnError:false in the changelog to avoid the following error when the script has already been executed:

Unexpected error running Liquibase: Migration failed for changeset src/test/resources/init-changelogs/gcp/mssql.sql::1::liquibase:
     Reason: liquibase.exception.DatabaseException: User, group, or role '***' already exists in the current database. [Failed SQL: (15023) EXEC sp_adduser '***']

@jnewton03
Copy link
Contributor

@jandroav would a cleaner solution be to check if the user already exists first? Something like:

USE lbcat;

IF NOT EXISTS (SELECT * FROM sys.sysusers WHERE name = 'lbuser')
BEGIN
    EXEC sp_adduser 'lbuser';
END

IF NOT EXISTS (SELECT 1 FROM sys.database_principals WHERE name = 'lbuser' AND type_desc = 'DATABASE_ROLE' AND is_fixed_role = 0)
BEGIN
    EXEC sp_addrolemember 'db_owner', 'lbuser';
END

@KushnirykOleh
Copy link
Contributor

KushnirykOleh commented Apr 24, 2023

@jandroav , after Pavlo's PR mssql on gcp seems to be working fine, i'm not sure if we need recreating user for test-harness tests

@jandroav
Copy link
Contributor Author

It seems not necessary and I can see it is working fine now. thanks!

@jnewton03
Copy link
Contributor

so should this pr be closed and ticket moved to done?

jandroav and others added 13 commits April 27, 2023 10:27
Bumps ojdbc8 from 21.9.0.0 to 23.2.0.0.

---
updated-dependencies:
- dependency-name: com.oracle.database.jdbc:ojdbc8
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [liquibase-github-actions/drop-all](https://github.com/liquibase-github-actions/drop-all) from 4.20.0 to 4.21.1.
- [Release notes](https://github.com/liquibase-github-actions/drop-all/releases)
- [Commits](liquibase-github-actions/drop-all@v4.20.0...v4.21.1)

---
updated-dependencies:
- dependency-name: liquibase-github-actions/drop-all
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
…teTest as a default test suite (#538)

* DAT-14478. replaced AdvancedHarnessSuiteTest with LiquibaseHarnessSuiteTest as a default test suite

* DAT-14478. refactored AdvancedHarnessSuiteTest to use older version of advanced test

* DAT-14478. reverted last commit

* DAT-14478. removed unused imports
…teTest as a default test suite. Fixes in test data for mssql:gcp. (#539)

* DAT-14478. replaced AdvancedHarnessSuiteTest with LiquibaseHarnessSuiteTest as a default test suite

* DAT-14478. refactored AdvancedHarnessSuiteTest to use older version of advanced test

* DAT-14478. reverted last commit

* DAT-14478. removed unused imports

* DAT-14478. changed init.sql for gcp mssql instance

* DAT-14478. changed init.sql for gcp mssql instance

* DAT-14478. fixed test data for gcp mssql harness suite

* DAT-14478. renamed mssql:2019 to mssql:gcp

* DAT-14478. small fix

* DAT-14478. small fix

* DAT-14478. fix in expeted sql for mssql gcp

* DAT-14478. fix in expeted sql for mssql gcp
Bumps [snowflake-jdbc](https://github.com/snowflakedb/snowflake-jdbc) from 3.13.29 to 3.13.30.
- [Release notes](https://github.com/snowflakedb/snowflake-jdbc/releases)
- [Changelog](https://github.com/snowflakedb/snowflake-jdbc/blob/master/CHANGELOG.rst)
- [Commits](snowflakedb/snowflake-jdbc@v3.13.29...v3.13.30)

---
updated-dependencies:
- dependency-name: net.snowflake:snowflake-jdbc
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
fix(mssql.sql): add newline at end of file
…angelog

feat(gcp.yml): add lbuser creation and db_owner role assignment in mssql.sql changelog
@jandroav
Copy link
Contributor Author

After fixing branch conflicts we need to merge our already tested change:

USE lbcat;

IF NOT EXISTS (SELECT * FROM sys.sysusers WHERE name = 'lbuser')
BEGIN
    EXEC sp_adduser 'lbuser';
END

IF NOT EXISTS (SELECT 1 FROM sys.database_principals WHERE name = 'lbuser' AND type_desc = 'DATABASE_ROLE' AND is_fixed_role = 0)
BEGIN
    EXEC sp_addrolemember 'db_owner', 'lbuser';
END

@PavloTytarchuk I could see GCP mssql tests are failing again with the same issue https://github.com/liquibase/liquibase-test-harness/actions/runs/4816521824/jobs/8576252537

@KushnirykOleh
Copy link
Contributor

@jandroav looking at this issue with @PavloTytarchuk we discovered most likely that dropAll doesn't work because of different schemas dbo (used in init script) and lbuser used in tests.
So recreating user won't resolve the issue.
We will try to figure out correct approach for gcp mssql as such config seems to be working differently that on AWS

@jandroav jandroav merged commit b82acf6 into main Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants