-
Notifications
You must be signed in to change notification settings - Fork 462
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
Use digest for sqlserver and fix tests #5059
Conversation
@@ -27,7 +27,8 @@ public static IResourceBuilder<SqlServerServerResource> AddSqlServer(this IDistr | |||
var sqlServer = new SqlServerServerResource(name, passwordParameter); | |||
return builder.AddResource(sqlServer) | |||
.WithEndpoint(port: port, targetPort: 1433, name: SqlServerServerResource.PrimaryEndpointName) | |||
.WithImage(SqlServerContainerImageTags.Image, SqlServerContainerImageTags.Tag) | |||
.WithImage(SqlServerContainerImageTags.Image) | |||
.WithImageSHA256(SqlServerContainerImageTags.Digest) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they suggested we should instead depend on digests as opposed to tags, to be able to guarantee determinism
Note that this is OK to unblock our tests, but this doesn't comply with our long term plan.
This approach doesn't allow for security patches to be automatically applied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's have a follow up discussion just to finalize on our plan going forward. We may very well end up following the same plan we have, I just want to make sure we take into consideration the concept of determinism and the fact that we have now been broken by this, so we should check what our customers would expect us to do.
For this particular case, it's worth noting that the underlying image doesn't follow semver which seems to be one of the keys of our plan.
An update to mssql/server image tag
2022-latest
broke our tests as we can no longer communicate with the container. Chatting offline with our experts from the dotnet-docker repo, they suggested we should instead depend on digests as opposed to tags, to be able to guarantee determinism, and then just make sure we update these digests in a frequent basis to ensure we get fixes for them. This PR is switching just the sqlserver hosting component to use digest instead and it also does that for the tests so that they pass again, but we will likely want to make an overhaul pass to all of our hosting components to similarly depend on digests as opposed to tags.cc: @mitchdenny @radical @eerhardt
Microsoft Reviewers: Open in CodeFlow