Skip to content

mssql-server: init at 15.0.4003.23-3 #80609

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

mssql-server: init at 15.0.4003.23-3 #80609

wants to merge 1 commit into from
+150 −0

Conversation

xavierzwirtz
Copy link
Contributor

Motivation for this change

Packages mssql-server for NixOS.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Sorry, something went wrong.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Feb 20, 2020
@aanderse
Copy link
Member

https://docs.microsoft.com/en-us/sql/linux/sql-server-linux-faq?view=sql-server-ver15

All files in the /var/opt/mssql file folder should be owned by the mssql user and belong to the mssql group. Both the mssql user and group should have read-write permissions of all files and directories.

@xavierzwirtz how does this work with the /nix/store?

@xavierzwirtz
Copy link
Contributor Author

xavierzwirtz commented Mar 1, 2020 •
edited
Loading

When I run sqlserver without sudo it blows up saying it doesnt have access to that directory:
/nix/store/r38dpv9wkd638nd5c9wmjp5ngbilkxdz-mssql-server-15.0.4003.23-3/opt/mssql/bin/sqlservr_old: Unable to open /var/opt/mssql/.system//instance_id: Permission denied (13)

However when run with sudo it launches and writes to /var/opt/mssql/. Not sure if that's acceptable, new to nix.

@aanderse
Copy link
Member

aanderse commented Mar 1, 2020

You should probably create a systemd service running as a dedicated mssql user and see if you can get the program to limit mutable data to somewhere like /var/lib/mssql.

I wonder if @jonringer has any experience running mssql on linux and could provide any information... 🤔

@xavierzwirtz
Copy link
Contributor Author

Unfortunately there are no config options to do that, it has to be done with NIX_REDIRECT. I did experiment with doing that, but something with the way sqlserver did file access made it where each file that it touched had to be created before hand.

jonringer
jonringer reviewed Mar 2, 2020
View reviewed changes
pkgs/servers/sql/mssql/server/default.nix Outdated
mv usr $out
fix_bash()
{
substituteInPlace $1 \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
substituteInPlace $1 \
substituteInPlace $@ \

and then you would be able to pass many file names:

fix_bash $out/opt/mssql/lib/mssql-conf/{invokesqlservr,checkinstall}.sh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at the source, I don't think I can do that:

substituteInPlace() {
    local fileName="$1"
    shift
    substitute "$fileName" "$fileName" "$@"
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Take a look at the latest version, I think it captures the spirit of this review.

Copy link
Contributor

Choose a reason for hiding this comment

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

I apologize, I really thought those helpers were able to take many arguments.

Don't really want to you churn on bike-shedding issues

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a problem, the comment showed me things I didn't know about glob expansion.

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 3, 2020
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 3, 2020
@aanderse
Copy link
Member

aanderse commented Mar 3, 2020

Seems like mssql is at least reasonably configurable to separate mutable state from binaries:

I assume that if properly configured mutable state and immutable binaries+config can be achieved. A systemd service configured via a NixOS module would be a very nice addition to this PR... 😄

@xavierzwirtz
Copy link
Contributor Author

Could I get some clarification on what you mean by separate mutable state from binaries? Is the problem that /var/opt/mssql/ is not where programs should be storing mutable state, and you want this wrapped up in a systemd service that configures mssql-server to store it in a more idiomatic place?

@aanderse
Copy link
Member

aanderse commented Mar 3, 2020

@xavierzwirtz from your earlier posts I was under the impression mssql was putting binaries and/or config into /var/opt/mssql but after going through your posts again it is possible I have misinterpreted this. Though to your point about a more idiomatic place the preference is always to store mutable state in something like /var/lib/mssql, logs in /var/logs/mssql, etc... as a default. On top of locations we can look at running this as a non root user, and applying some security features of systemd on top of it.

@xavierzwirtz
Copy link
Contributor Author

No binaries in /var/opt/mssql, it is storing config though. Trying to be very specific, only packaged simple things before. I will take a stab at using NIX_REDIRECTS to move where mssql-server is storing that data again; the environment variables they expose only allow you to move database files, not the system configuration.

@xavierzwirtz xavierzwirtz force-pushed the mssql-server branch 2 times, most recently from 0d1b309 to 25008d4 Compare April 30, 2020 03:33
@xavierzwirtz
Copy link
Contributor Author

Experimented some more with using NIX_REDIRECTS to move the database and config files out of /var/opt/mssql. It's doing something that libredirect.c doesn't currently support though, and I haven't been able to figure out what that is by fiddling with the wires. strace logging here.

@xavierzwirtz xavierzwirtz force-pushed the mssql-server branch 2 times, most recently from 0cebb1d to d3429b3 Compare May 1, 2020 06:35
@ryantm ryantm added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Oct 3, 2020
@ryantm ryantm added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Oct 3, 2020
@stale
Copy link

stale bot commented Jun 7, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 7, 2021
@onny onny marked this pull request as draft September 7, 2022 06:41
@onny
Copy link
Contributor

onny commented Sep 7, 2022

Upstream released version 15.0.4249.2

@Janik-Haag Janik-Haag added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 12, 2023
@abathur
Copy link
Member

abathur commented Aug 27, 2023

@xavierzwirtz Are you still interested in pursuing this?

@abathur
Copy link
Member

abathur commented Sep 4, 2023

Tentatively closing. Happy to reopen if you're still interested.

@abathur abathur closed this Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonringer jonringer

Assignees
No one assigned
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@xavierzwirtz @aanderse @onny @abathur @jonringer @ryantm @Janik-Haag