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

Allow port forwarding visibility to be specified #5

Open
asciimike opened this issue Jan 21, 2022 · 12 comments
Open

Allow port forwarding visibility to be specified #5

asciimike opened this issue Jan 21, 2022 · 12 comments
Labels
proposal Still under discussion, collecting feedback

Comments

@asciimike
Copy link

Currently developers can specify which ports to forward, but they can't specify visibility of those ports in e.g. Codespaces, so users are manually setting them every time, which is a bit of a pain.

I would propose adding a visibility: public | org | private option to portsAttributes:

{
    ...
    "forwardPorts": [3000],
    "portsAttributes": {
        "3000": {
	    "visibility": "public"
	}
    }
    ...
}

C.f. community/community#10394

@Chuxel Chuxel added the proposal Still under discussion, collecting feedback label Jan 21, 2022
@Chuxel
Copy link
Member

Chuxel commented Jan 31, 2022

I think the question is what the values should be here if we keep this as a general devcontainer.json property. The values public verses private can be thought about as listening on "All interfaces" while private is just localhost (and the cloud equivalents therein). So those are clean.

The value of org doesn't make sense in other contexts. Perhaps it could possibly be something like restricted. It could then mirror the way scopes are defined using : to handle the service/tool specific version.

e.g. restricted:codespaces:org

The value of codespaces would mirror the final call for tool/specific properties for #1. The alternative would be that the specific restriction could go as a property under there instead - but I understand the likely desire for a short hand. In the local case, I'd assume we'd want restricted would just mirror private, but other tools/services could have a slightly different take on it.

Thoughts @chrmarti @bamurtaugh ?

@bamurtaugh
Copy link
Member

The values public | restricted | private make sense to me. I could also see limited, set, defined, or assigned as another option for restricted, as these sound like middle grounds between public and private, and could read well when the user sets a value.

@chrmarti
Copy link
Contributor

chrmarti commented Feb 1, 2022

/cc @alexr00

@alexr00
Copy link

alexr00 commented Feb 1, 2022

I lean towards restricted. However, it seems confusing to make public mean "all interfaces" with Remote Containers and "actually anyone with a link" in Codespaces.

Can visibility be something that is Codespaces specific?

@Chuxel
Copy link
Member

Chuxel commented Feb 1, 2022

I can see the value in enabling an all interfaces option locally - it would allow you to hit the ports from another machine. By default, you'd have private that is just localhost. Certainly we could have a different name than public, but it does describe what happens pretty well - it's available to anyone on the same network as your machine.

I suppose shared would be an alternative to public since it's not always going to be on the public internet. So, values could be shared, restricted, private.

@alexr00
Copy link

alexr00 commented Feb 2, 2022

Limiting the visibility to 3 choices is also artificial. Codespaces can have as many visibility options as they want. Will we consider having additional options if Codespaces changes their options?

@bamurtaugh
Copy link
Member

With the effort in #1, perhaps different providers could provide different visibility options? They could all be called portsAttributes, but "codespaces" port visibility values could be different than those in "vscode" or another provider for instance.

@asciimike
Copy link
Author

I tend to like @Chuxel's framing of public and private being common and then having namespaced options specific to providers, as this would let us add our own new options without having to get it approved explicitly (e.g. codespaces:team:github/codespaces which would enable team level visibility, if/when we wanted to offer such a concept).

I wouldn't be opposed to having everything be namespaced as well, and leaving it up to each consumer to implement the appropriate behavior.

@Chuxel
Copy link
Member

Chuxel commented Feb 2, 2022

Hmmm. Yeah private / public does seem like something that is desirable to do as a core capability, the question is how to best support the extensibility.

@alexr00, agreed that three is arbitrary. I was more thinking the third option is more "delegate to the specified tool" like @asciimike was saying. Perhaps what we need to do here is support an object notation for more advanced scenarios that is optional. To @bamurtaugh 's point, you could then do different privs by product.

So, we could have:

"visibility": "private"
"visibility": "public"
"visibility": [{
    "product": "codespaces",
    "access": ["team:github/codespaces"]
}, {
    "product": "remoteContainers",
    "access": ["public"]
}]

Or something like that... the "product" name would mirror #1. At that point "visibility":"public" is short hand for a product value of "all"?

This is an interesting one since the property itself is common, but the values could be product / tool specific. #1 was really about us thinking how to move truly product specific properties into a more extensible form.

@alexr00
Copy link

alexr00 commented Feb 10, 2022

Including the "product" name in the setting would make it clearer, but then we would be diverging from the schema that VS Code uses for visibility in the remote.portsAttributes setting, which also seems undesirable.

I would prefer shared/private/other strings that devcontainer.json supporters interpret however they want over diverging from the existing schema.

@chrmarti
Copy link
Contributor

I think keeping product-specific properties limited to the top-level properties we discuss in #1 is preferable for now. This will keep handing-off the JSON schema fragments to the corresponding products simple.

@cnuss
Copy link

cnuss commented Apr 26, 2024

Hey there! Did we get quorum on this feature request? I'm extremely interested in it as I've had to hack out a solution for making ports public at startup and its very ugly.

I basically have a watchdog process that runs the following command at startup:

@cnuss ➜ /workspaces/stack-aws-serverless-express (feat/s3) $ gh codespace ports visibility -c $CODESPACE_NAME 4566:public
* Request at 2024-04-26 12:39:56.033648403 +0000 UTC m=+0.048657246
* Request to https://api.github.com/user/codespaces/automatic-pancake-9799vw6jp62xj6r?internal=true&refresh=true
* Request took 275.169517ms
* Request at 2024-04-26 12:39:56.309781899 +0000 UTC m=+0.324790742
* Request to https://use.rel.tunnels.api.visualstudio.com/api/v1/tunnels/swift-chair-hcb4g9h?includePorts=true
Connecting to codespace ⣾* Request took 45.084589ms
* Request at 2024-04-26 12:39:56.35543031 +0000 UTC m=+0.370439133
* Request to https://use.rel.tunnels.api.visualstudio.com/api/v1/tunnels/swift-chair-hcb4g9h/ports/4566?includePorts=true
Updating port 4566 visibility to: public ⣾* Request took 16.548733ms

Here's my 2-cents...
It appears that gh codespace ports visibility basically calls out to tunnels.api.visualstudio.com/api/v1, so, I'm thinking the visibility value in the devcontainer.json spec could simply be a string, which defers interpolation of the allowed values to be whatever the underlying API supports.

For most cases its public/private, but for GHE its public/private/organization.

    "forwardPorts": [3000],
    "portsAttributes": {
        "3000": {
	    "visibility": "WHATEVER_THE_UNDERLYING_API_ALLOWS"
	}
    }

Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Still under discussion, collecting feedback
Projects
None yet
Development

No branches or pull requests

6 participants