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.

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

app settings.json not honoring database configuration #197

Open
RachelAmbler opened this issue Nov 27, 2024 · 5 comments · May be fixed by #206
Open

app settings.json not honoring database configuration #197

RachelAmbler opened this issue Nov 27, 2024 · 5 comments · May be fixed by #206
Assignees
@Regenhardt
Labels
question Further information is requested

Comments

@RachelAmbler
Copy link

RachelAmbler commented Nov 27, 2024 •
edited
Loading

Runningbagetter as a docker container

Describe the bug

appsettings.json not honoring database configuration. Unsure yet if it's honoring other sections.

To Reproduce

{
  "PackageDeletionBehavior": "Unlist",
  "AllowPackageOverwrites": false,
  "MaxPackageSizeGiB": 8,
  "Database": {
    "Type": "PostgreSql",
    "ConnectionString": "Server=myDbServer;Port=5432;Database=bagetter;User Id=bagetter;Password=myPassword"
  },
  "Storage": {
    "Type": "FileSystem",
    "Path": ""
  },
  "Search": {
    "Type": "Database"
  },
  "Mirror": {
    "Enabled": true,
    "PackageSource": "https://api.nuget.org/v3/index.json"
  },
  "Logging": {
    "IncludeScopes": false,
    "Debug": {
      "LogLevel": {
        "Default": "Warning"
      }
    },
    "Console": {
      "LogLevel": {
        "Microsoft.Hosting.Lifetime": "Debug",
        "Default": "Warning"
      }
    }
  },
  "HealthCheck": {
    "Path": "/health"
  },
  "Statistics": {
    "EnableStatisticsPage": true,
    "ListConfiguredServices": true
  },
  "Authentication": {
    "ApiKeys": [
      {
        "Key": "myApiKey"
      }
    ],
    "Credentials": [
      {
        "Username": "Admin",
        "Password": "myPassword2"
      }
    ]
  }
}

Expected behavior

For BaGetter to connect to my PostgreSql server

Additional context

It connects just fine if I use environment variables. I know the appsettings.json file is being read because if I change one of the Logging options to an invalid logging type (e.g. Invalid) then I get exceptions as a result.

To further debug, I also changed ConnectionString to xconnectionString - this should also result in an exception as ConnectionString is decorated with the [Required] attribute. However no such event happens.

I'm also unsure if the auth piece is working as I've yet to get to that section but, as it stands, I'm able to hit the web page directly with no auth challenges.

docker-compose.yml:

services:
  NGinx:
    container_name: baGetter-Nginx
    image: nginx:latest
    restart: unless-stopped
    networks:
      - baGetterNet
    ports:
      - "1443:1443"
    volumes:
        - ./nginx/nginx.conf:/etc/nginx/nginx.conf:ro
        - /etc/ssl/certs/CorpBundle.pem:/etc/ssl/certs/site.crt
        - /etc/ssl/private/CorpPrivateKey.pem:/etc/ssl/private/site.key
        - ./nginx/private/PrivateKey-Protected.key:/etc/ssl/private/site.fifo
        - ./nginx/access.log:/var/log/nginx/access.log
        - ./nginx/error.log:/var/log/nginx/error.log
    depends_on:
      - baGetter
  baGetter:
    container_name: baGetter
    image: bagetter/bagetter:latest
    volumes:
      - data:/data
      - ./appsettings.json:/app/appsettings.json
    restart: always
    networks:
      - baGetterNet
volumes:
  data:
networks:
  baGetterNet:
@Regenhardt
Copy link

Could there be environment variables still on the system that bagetter can read? The ASP.NET Core runtime will take any environment variables and override whatever is in the application.json file, so modifying the file wouldn't have any effect.

An old idea could help analyse this, let me create a feature release, maybe even today, so we can see what's going on.

@RachelAmbler
Copy link
Author

This was clean out the box from today - as you can see my docker-compose.yml had nothing listed at the time. I even destroyed the image to see if that helped.

Building the project locally does show it is reading the appsettings.json file and failing when I rename the ConnectionString key. So I'm a little confused as to what's going on.

As it stands now I can manage a combo of using both appsettings.json and environment variables defined in my docker-compose.yml so it's not a show stopper for me - but it did cause no end of confusion for me today.

@Regenhardt
Copy link

Ok so I found the problem and I guess we should definitely add that to the docs somewhere.

The docker image we build automatically sets sqlite set as database configuration via env variables. This means that any appsettings.json modifications you add to the container will be overridden by ASP.NET Core reading those default environment variables from the container.

We can definitely get rid of that, or rather comment it out so it still stands as an example for people wanting to build their own image. For now, you'll have to rely on the Database.Type and Database.ConnectionString being injected via the environment.

@Regenhardt
Copy link

Need opinions and ideas:

appsettings.json sets the default db file to bagetter.db.
Dockerfile sets it to /data/db/bagetter.db.

This means, everybody using the docker image without configuring their own DB is using /data/db/bagetter.db right now.
Everybody running the application without the docker image, i.e. built themselves or downloaded the release (linux or windows) is using ./bagetter.db right now.

Just removing the default env from the Dockerfile would break unmodified docker installations by changing the default db location.
Removing the env from the Dockerfile and changing the default appsettings.json would break not only docker-less installs but also windows installs (all this applies to unmodified db settings of course). Also development on windows now needs the appsettings.Development.json file to override it back to a windows-usable relative path.
Also linux docker-less installs would suddenly use an absolute path, breaking existing installs and also potentially breaking out-of-the-box for using a potentially non-existant or non-permitted /data directory.

So development can be fixed by modifying the appsettings.Development.json either way.

Any ideas about how to remove the default env from the Dockerfile without breaking both existing installations and both existing and future unmodified windows installations?

Or do we just add setting the DB via environment as necessary for docker installs?

@Regenhardt Regenhardt added the question Further information is requested label Nov 28, 2024
@joestr
Copy link

joestr commented Dec 2, 2024 •
edited
Loading

Or do we just add setting the DB via environment as necessary for docker installs?

This is the way to go in my opinion. Not breaking the current behaviour. A clear mention in the documentation should be enough to reference it later if the question comes up anytime in the future.

@Regenhardt Regenhardt self-assigned this Jan 1, 2025
Regenhardt added a commit to Regenhardt/BaGetter that referenced this issue Jan 2, 2025
@Regenhardt
docs: Improve docker hosting information
58c1089 
bagetter#205, bagetter#197
Regenhardt added a commit to Regenhardt/BaGetter that referenced this issue Jan 2, 2025
@Regenhardt
docs: Improve docker hosting information
525a4d6 
bagetter#205, bagetter#197
@Regenhardt Regenhardt linked a pull request Jan 2, 2025 that will close this issue
Open
@Regenhardt Regenhardt linked a pull request Jan 2, 2025 that will close this issue
docs: Improve docker hosting information #206
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Regenhardt Regenhardt

Labels
question Further information is requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

docs: Improve docker hosting information Regenhardt/BaGetter
3 participants
@RachelAmbler @Regenhardt @joestr