bagetter / BaGetter

A lightweight NuGet and symbol server
https://www.bagetter.com
MIT License
273 stars 57 forks source link

app settings.json not honoring database configuration #197

Open RachelAmbler opened 3 days ago

RachelAmbler commented 3 days ago

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 commented 3 days ago

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 commented 3 days ago

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 commented 2 days ago

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 commented 2 days ago

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?