Restore foreign keys and add constraints

[vsychov]

Hello there,

This pull request addresses the issue #1482, aiming to fix the anomalies regarding foreign keys and constraints in database models. Initially FK was disabled here: #41 without any description why it was done.

These changes improve data integrity, maintain referential integrity between different tables, and ensure that the application's behavior remains consistent and predictable when handling database records, particularly in delete operations.

https://gorm.io/docs/gorm_config.html#DisableForeignKeyConstraintWhenMigrating

Fixes #1482

• read the CONTRIBUTING guidelines
• raised a GitHub issue or discussed it on the projects chat beforehand
• updated CHANGELOG.md

[kradalby]

Looks reasonable to me, I'll let @juanfont have the last say since he did the change, but I recon it would be hard to remember.

[vsychov]

Hello, @juanfont , @kradalby, I was wondering if there have been any updates regarding the merge this?

[kradalby]

Hi, I'm had some other work the previous weeks and I'm on holiday this week but hope to get back to the last things next week.

[vsychov]

Hey @kradalby, could you check this out? I just resolved conflicts, and it seems like the tests are running smoothly now.

[kradalby]

I think this looks reasonable, I'm running the tests now.

Are there any potential concerns we should think of that might not be covered by the current tests?

[vsychov]

@kradalby , I couldn't think of any potential issues that might arise with this, and for which there are no tests already in place, the changes seem fairly safe.

[vsychov]

@kradalby, I see that all tests passed except TestAuthKeyLogoutAndRelogin, which failed with the error:

general_test.go:165: failed to have all clients sync up: reached retry deadline: [ts-1-40-muk9lb] peer count correct, but ts-head-pkmesn does not have a DERP

and

general_test.go:165: failed to have all clients sync up: [ts-1-40-rottie] peer count correct, but ts-head-roffmn does not have a DERP

From what I understand, there seems to be an issue with DERP. Overall, is this test stable? Is there a chance that this instability is due to infrastructure issues with the tests, or is the test error caused by my changes specifically? Should I investigate further to find the root cause?

[kradalby]

I'm seeing that particular test fail on one of my PRs too, but not locally, not sure what is up.

[vsychov]

I'm seeing that particular test fail on one of my PRs too, but not locally, not sure what is up.

So, can this PR be merged? If yes, let's do it please, to prevent future conflicts.

[vsychov]

@kradalby, I found the reason why it works locally for you but not in pipelines. The issue lies in something broken in the Docker image of tailscale head and unstable, causing the Peer.*.Relay field to be empty in the clients status for these versions. Locally, you probably have a cached older version of this images, so the test works locally.

It might be worth reporting this in the issues section of tailscale. Since you're presumably affiliated with the tailscale team, perhaps you could inform them about this issue, as I don't have a full understanding of the reasons to describe the issue accurately.

Here's a snippet from the contents of ts-1-60-q8fnnl_status.json:

...
  "Peer": {
    "nodekey:04ac28657e6c28a0a71ab4c60e96d32921908e413de278eecae8693aff589445": {
      "HostName": "ts-unstable-ntcuuv",
      "Relay": "",
      ...
    },
    "nodekey:0de113990c6918c86cde623ec129c5977e5c904ecd80e4d9849bbb909fbfff04": {
      "HostName": "ts-1-40-kcnou1",
      "Relay": "par",
      ...
    },
    ...
}

I've also included a commit with a fix that adjusts the makefile command to run integration tests: 18bd635

[kradalby]

I have not had too much time to look into it, but after clearing the cache I can confirm this behaviour, however, I dont get why I dont see it with any of the other tests because they have the same assertion.

[kradalby]

What I find odd is that it all the other tests pass, and those two consistently fail. I've poked around in the tailscale client code and it doesnt seem like there is any change that should affect this.

[vsychov]

I went through the versions of Tailscale and managed to find the version with the issue, unstable-v1.65.188 (May 8, 2024 at 5:46 am) - this version has a problem. The previous version, unstable-v1.65.179 (May 7, 2024 at 2:30 am), works fine. So, the issue likely occurred somewhere between these versions. Unfortunately, I don't know how to link the Docker image version to a tailscale Git commit, but I hope this helps in troubleshooting.

Here's the link to the Docker Hub page with the versions

[kradalby]

I'll take a look, thanks, since it is not related to this PR, I will merge it for now.