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

Update RedGate worker friendly templates with new functionality #1555

Merged
merged 3 commits into from
Sep 6, 2024

Conversation

markgould
Copy link
Contributor

@markgould markgould commented Sep 5, 2024

Background

  • Adds Trust Server Certificate to Create Database Release (Worker Friendly) and Deploy Database Release (Worker Friendly) to be consistent with Deploy From Package (Worker Friendly)
  • Adds ability to pass a custom connection string primarily to allow for additional authentication options supported in Microsoft.Data.SqlClient

Results

Existing behavior still works

After

image

Pre-requisites

  • Id should be a GUID that is not 00000000-0000-0000-0000-000000000000
    • NOTE If you are modifying an existing step template, please make sure that you do not modify the Id property (updating the Id will break the Library sync functionality in Octopus).
  • Version should be incremented, otherwise the integration with Octopus won't update the step template correctly
  • Parameter names should not start with $
  • Step template parameter names (the ones declared in the JSON, not the script body) should be prefixed with a namespace so that they are less likely to clash with other user-defined variables in Octopus (see this issue). For example, use an abbreviated name of the step template or the category of the step template).
  • LastModifiedBy field must be present, and (optionally) updated with the correct author
  • The best practices documented here have been applied
  • If a new Category has been created:
    • An image with the name {categoryname}.png must be present under the step-templates/logos folder
    • The switch in the humanize function in gulpfile.babel.js must have a case statement corresponding to it

@markgould
Copy link
Contributor Author

Not sure why the formatting is so off - json files edited in VSCode. Any suggestions?

@markgould
Copy link
Contributor Author

@twerthi @BobJWalker since you've both contributed here before open to your feedback.

The new connection string parameter bypasses the New-DatabaseConnection command and instead instantiates RedGate.Versioning.Automation.Compare.SchemaSources.DatabaseConnection to pass a custom connection string. New-DatabaseConnection supports a very limited subset of Authentication values, this seemed the best way to handle this.

@hnrkndrssn
Copy link
Contributor

Hey @markgould it looks like it might be a spaces vs tabs issue causing the amount of changes. Hiding white space in the file view makes only the actual changes to show up.

@twerthi
Copy link
Contributor

twerthi commented Sep 5, 2024

That sounds reasonable to me. As you're probably aware, Redgate is opting to invest in the Flyway database migrations product vs. the SQL Automation product so these templates may receive reduced usage.

@markgould
Copy link
Contributor Author

markgould commented Sep 6, 2024

That sounds reasonable to me. As you're probably aware, Redgate is opting to invest in the Flyway database migrations product vs. the SQL Automation product so these templates may receive reduced usage.

Yeah - we have it on our roadmap to transition at some point but have lots of projects using it today so it will take some time for us to migrate. Hopefully this can help others that may still be using the Change Automation tools.

@markgould
Copy link
Contributor Author

Hey @markgould it looks like it might be a spaces vs tabs issue causing the amount of changes. Hiding white space in the file view makes only the actual changes to show up.

Could be, looks like some of the formatting may have been off previously too. Any changes you'd like me to make here?

@twerthi
Copy link
Contributor

twerthi commented Sep 6, 2024

@markgould There is an error in the template Redgate - Deploy from Package (Worker Friendly) when not using the Connection String (Optional) parameter. The template is using the PowerShell variable $DLMAutomationDatabaseServer where I believe it is meaning to use the $DLMAutomationTargetDatabaseServer parameter?

I didn't check the others, would you mind checking the other two for a similar situation?

@twerthi
Copy link
Contributor

twerthi commented Sep 6, 2024

Looks to be a similar issue with the database name, the code is using $DLMAutomationDatabaseName whereas the Parameter is $DLMAutomationTargetDatabaseName. After updating that one in my testing, I have encountered other issues. Did you update some Parameter names in the steps?

@markgould
Copy link
Contributor Author

I see what's going on here - there are discrepancies with the parameter names in the deploy-from-package vs the other two, so when I updated that one there was an issue with it using the wrong variable names. Correcting now.

…lues. Fix variable name issues in Deploy From Package.
@markgould
Copy link
Contributor Author

Should be good now. I also noticed an issue in how I was checking the Trust Server Certificate value across the 3 scripts, changed to use Convert.ToBoolean to handle empty values. Thanks for the feedback!

Copy link
Contributor

@twerthi twerthi left a comment

Choose a reason for hiding this comment

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

LGTM

@twerthi twerthi merged commit 439408c into OctopusDeploy:master Sep 6, 2024
2 checks passed
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.

3 participants