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.

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

[APIv2] secrets are missing Version field #9316

Closed
riyad opened this issue Feb 10, 2021 · 9 comments · Fixed by #9583
Closed

[APIv2] secrets are missing Version field #9316

riyad opened this issue Feb 10, 2021 · 9 comments · Fixed by #9583
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@riyad
Copy link
Contributor

riyad commented Feb 10, 2021

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind bug

Description

Trying to find regressions by exercising the APIv2 trough docker-py's test suite (see #5386) I came across a failing assertion SecretAPITest::test_inspect_secret test.

It seems secrets returned from GET /secrets and GET /secret/{id} endpoints are missing the (non-optional) Version field.

Steps to reproduce the issue:

  1. create a secret
$ curl -sS --unix-socket /var/run/user/1000/podman/podman.sock -XPOST 'http://localhost/v1.40/secrets/create' -H "Content-Type: application/json" -d '{"Data": "Zm9vIGJhcg==", "Name": "dockerpy_test1"}'
{"ID":"f76cb9091ea531994cd9e9438"}
  1. inspect the secret
$ curl -sS --unix-socket /var/run/user/1000/podman/podman.sock -XGET 'http://localhost/v1.40/secrets/f76cb9091ea531994cd9e9438' 
{"ID":"f76cb9091ea531994cd9e9438","CreatedAt":"2021-02-10T23:57:25.547915167+01:00","UpdatedAt":"2021-02-10T23:57:25.547915167+01:00","Spec":{"Name":"dockerpy_test1","Driver":{"Name":"file","Options":null}}}

Describe the results you received:

The secrets returned don't have a Version field.

Describe the results you expected:

The secrets returned should have a Version field.

see Docker API docs: https://docs.docker.com/engine/api/v1.40/#operation/ConfigInspect

@openshift-ci-robot openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Feb 10, 2021
@mheon
Copy link
Member

mheon commented Feb 11, 2021

@ashley-cui PTAL

@ashley-cui
Copy link
Member

hmm, didn't we decide that it doesn't make sense to have a version field since docker only responds with a version field since it has a swarm manager that deals with secrets? but i guess since its compat, should i wire in the version number anyway?

@vrothberg
Copy link
Member

hmm, didn't we decide that it doesn't make sense to have a version field since docker only responds with a version field since it has a swarm manager that deals with secrets? but i guess since its compat, should i wire in the version number anyway?

Yes, that what we thought initially. I am also not sure which version to return. @jwhonce @baude WDYT?

@rhatdan
Copy link
Member

rhatdan commented Feb 11, 2021

I have no idea what version to return, Perhaps API version.

@mheon
Copy link
Member

mheon commented Feb 11, 2021

Docker secrets allow mutation - AFAIK the Version is incremented every time the secret changes.

It sounds like we should add the field and fix it to 1?

@ashley-cui
Copy link
Member

Ohh that makes more sense than the swarm version number. I can add the version in pretty easily, I can do that fairly easily, if it's just fixing it to 1

@vrothberg
Copy link
Member

42 would also be cool 😎

@ashley-cui
Copy link
Member

ashley-cui commented Feb 22, 2021

Update: I tested it out and creating a secret defaults version to 11. I can't figure out what 11 means, and I also can't find where it the docs that it explains what the secret version means. If anyone recalls if they read anything about versions, let me know :)

@mheon @vrothberg @riyad

@vrothberg
Copy link
Member

@ashley-cui, there is a secret update endpoint which allows for updating a given secret. AFAIUI, the version number is like a revision number which is used to avoid conflicting writes.

Without having looked too closely, I assume that an update only works when the specified version is v+1 with the local version of the matching secret being v. This addresses race conditions in the kind of version protocol.

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants