feat: introduce secret URLs to runner configuration #1383
No reviewers
Labels
No labels
FreeBSD
Kind/Breaking
Kind/Bug
Kind/Chore
Kind/DependencyUpdate
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
Windows
linux-powerpc64le
linux-riscv64
linux-s390x
run-end-to-end-tests
run-forgejo-tests
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
forgejo/runner!1383
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "aahlenst/runner:secret-urls"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Introduce secret URLs to the runner configuration. They enable loading secrets from files.
cache.secretandserver.connections.<name>.tokeneach got a companion field:cache.secret_urlandserver.connections.<name>.token_url, respectively. They are mutually exclusive. For example, ifcache.secretandcache.secret_urlare both present, then Forgejo Runner will exit with an error.Shortened example configuration:
file:URLs can optionally include a single placeholder:$CREDENTIALS_DIRECTORY. If an environment variableCREDENTIALS_DIRECTORYcan be found,$CREDENTIALS_DIRECTORYwill be replaced by its value. If no environment variableCREDENTIALS_DIRECTORYis present,$CREDENTIALS_DIRECTORYwill not be replaced.Example unit file that demonstrates integration with systemd Credentials:
Corresponding config.yml:
See forgejo/forgejo-actions-feature-requests#88 for motivation and background.
I'm not 100% sure about
secret:. The alternative would be to interpret everything without a scheme as plain secret. On the other hand, I don't think that ambiguities are good.cc @viceice: Hope the feature works for you. 😄
I don't love
secret:....If we wanted to make these values consistently "always URLs", then it would imply to me that they should be
data:...as that's a well-known URL scheme with inline data. However, it's obviously ugly here --data:,some-secret-value, ordata:text/plain,some-secret-valuewould be the standardized format, and then characters need to be percent-encoded or base64 encoded (data:text/plain;base64,...).I'd prefer just omitting the URL scheme.
secret:confuses more than it clarifies (to me), anddata:isn't very usable for manual entry.I'm also confused about the usage of
file:-- it doesn't seem consistent with standard expectations of a file URL. A file URL has a host component that can be omitted. I'd expectfile:///$CREDENTIALS_DIRECTORY/runner-token, notfile:$CREDENTIALS_DIRECTORY/runner-token?@mfenniak wrote in #1383 (comment):
I'm happy to ditch
secret:. The question then is: what's the alternative? Please see forgejo/forgejo-actions-feature-requests#88 (comment) for the existing proposals.Note that I am talking about URIs (i at the end), not URLs (l at the end). I haven't studied URLs, so I don't know whether there are differences.
I looked into https://www.rfc-editor.org/rfc/rfc3986#section-1.1.1 and https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Syntax. It looks like the authority is optional. The first two slashes are only required when there is an authority. Therefore,
file:/my/path/hereis valid as would byfile:hello-there.txt. The tests cover both cases.url.Parse()seems to agree.Now, I interpret this as another indication that using URIs is not a good idea at all. You're confused, I had to read RFCs and paused when typing
secret: secret:verysecret.That would leave us with separate variables:
cache.secretandserver.connections.<name>.tokenwould work as before.What should the behaviour be if both are present? I'd raise an error instead of introducing precedence rules that are hard to discover.
You're correct, the authority section of the file URL is optional.
Multiple fields (
secretandsecret_file) may be equally good when there are two options, but I think if we have any future desire to expand this it becomes very messy. 🤔 I'm imagining supporting a remote secret store (AWS SAM, Vault, etc.), and having three or more options.What if the two fields were
secretandsecret_url? This meets a couple goals:secret_urlcleanlyOr alternatively... we add the URL capability as a child field?
$CREDENTIALS_DIRECTORYis also going to raise a separate design question. It makes it look to a user that environment variables can be used, but only a single one is supported. 🤔 I think this is probably fine but wonder if there's anything we can do to minimize user confusion.@mfenniak wrote in #1383 (comment):
I find future-proofing difficult, especially if I don't know anything about integrating secret stores like Vault. If you think it's a good idea, then I'm fine with using
secret_url.I'd like to reserve that option for a potential introduction of secret stores.
%credentials_directory? Ultimately, there's only so much we can do.secret_url&token_urlas a separate field, with an error if both are provided. 👍$CREDENTIALS_DIRECTORYas currently implemented. 👍9adc3c5e138103b426c7I went with
secret_uriinstead ofsecret_urlto be extra specific before somebody thinks they can use something likesecret_url: https://example.com/path/to/secret.txt. Otherwise, it should behave as discussed.I updated the PR description and
forgejo-runner generate-configaccordingly. Each stresses that$CREDENTIALS_DIRECTORYis a placeholder and that it is the only supported placeholder.@aahlenst wrote in #1383 (comment):
I don't think we share the same understanding of the difference between a URI and a URL. I'll describe my understanding:
URIs are the vaguest of the uniform resource things. They provide a unique identifier for a resource. URIs can be references that aren't functional to locate a resource, but just identify it; like
urn:oasis:names:specification:docbook:dtd:xml:4.1.2.URLs are are subset of URIs, which provide a scheme that can be used to locate a resource, hence they are uniform resource locators.
By making
secret_uri:https://example.com/path/to/secret.txtis both a URI and a URL, and so if you're concerned about people misunderstanding that it can be used here, that misunderstanding still existsIf we're in a disagreement here, let's get a third opinion involved.
server.connectionsconfig to poll multiple Forgejo servers simultaneously #13808103b426c7c4617fd1e0c4617fd1e037d3688d5afeat: introduce secret URIs to runner configurationto feat: introduce secret URLs to runner configuration@mfenniak wrote in #1383 (comment):
I don't think that is has to come to this. 😆
PR updated, description updated, commit message updated.
cascading-pr updated at actions/setup-forgejo#892