Skip to content

Conversation

@roman-y-wu
Copy link

…secret names

What I did

  • Modified configuration.go to deduplicate secret names using a map before reading them from Docker Desktop.
  • Added warning logging in clientpool.go for missing secrets. The root cause was actually the duplicate secret names bug, which is now fixed.

Related issue

closes #91 #86

(not mandatory) A picture of a cute animal, if possible in relation to what you did

@roman-y-wu roman-y-wu requested a review from a team as a code owner August 18, 2025 05:45
Copy link
Contributor

@kgprs kgprs left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Just a few small changes

Comment on lines +417 to +418
for name := range uniqueSecretNames {
secretNames = append(secretNames, name)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can be just secretNames := maps.Keys(uniqueSecretNames)

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for the comments. It has been updated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome, I think you missed this comment where we can replace the for-loop though secretNames := maps.Keys(uniqueSecretNames) or maybe forgot to push it

@kgprs kgprs added good first issue Good for newcomers and removed good first issue Good for newcomers labels Aug 18, 2025
Comment on lines +417 to +418
for name := range uniqueSecretNames {
secretNames = append(secretNames, name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome, I think you missed this comment where we can replace the for-loop though secretNames := maps.Keys(uniqueSecretNames) or maybe forgot to push it

Copy link
Collaborator

@slimslenderslacks slimslenderslacks left a comment

Choose a reason for hiding this comment

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

Let's merge this

@slimslenderslacks slimslenderslacks merged commit 08a81a4 into docker:main Aug 23, 2025
5 checks passed
masegraye pushed a commit to masegraye/docker-mcp-gateway that referenced this pull request Aug 23, 2025
docker#95)

* Improve secret handling by logging missing secrets and deduplicating secret names

* Refactor secret deduplication to use struct for improved memory efficiency
null-runner pushed a commit to null-runner/mcp-gateway that referenced this pull request Dec 6, 2025
docker#95)

* Improve secret handling by logging missing secrets and deduplicating secret names

* Refactor secret deduplication to use struct for improved memory efficiency
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error reading secrets when two servers are using the same secret name

3 participants