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

"caddy add-package" could allow adding by version #6549

Closed
gedw99 opened this issue Aug 29, 2024 · 10 comments · Fixed by #6665
Closed

"caddy add-package" could allow adding by version #6549

gedw99 opened this issue Aug 29, 2024 · 10 comments · Fixed by #6665

Comments

@gedw99
Copy link

gedw99 commented Aug 29, 2024

At the moment we cant control the version of the package added ?

The golang module system could allow us to add by tag or branch.

I expect many developers would want to use a tag ( as in git tag ) to help remediate supply channel attacks or other things of this nature.

caddy add-package -h

Downloads an updated Caddy binary with the specified packages (module/plugin)
added. Retains existing packages. Returns an error if the any of packages are
already included. EXPERIMENTAL: May be changed or removed.

Usage:
  caddy add-package <packages...> [flags]

Flags:
  -h, --help          help for add-package
  -k, --keep-backup   Keep the backed up binary, instead of deleting it

This would bring it inline with the list-modules --packages --versions command, which is version aware, so that its all consistent and all commands are version aware.

list-modules --packages --versions

Standard modules: 121

exec v0.0.0-20240603212820-a42a5b2ae10f github.com/abiosoft/caddy-exec
http.handlers.exec v0.0.0-20240603212820-a42a5b2ae10f github.com/abiosoft/caddy-exec

  Non-standard modules: 2

There is no point making caddy remove-package version aware since the binary can only have one version of a package.

final look and feel

For example, its is currently:


caddy add-package -h

Downloads an updated Caddy binary with the specified packages (module/plugin)
added. Retains existing packages. 

Returns an error if the any of packages are
already included. EXPERIMENTAL: May be changed or removed. 

with versioning it becomes :


caddy add-package -h

Downloads an updated Caddy binary with the specified packages (module/plugin)
added. Retains existing packages. 

Returns a success if the any of package versions are changed that are
already included. EXPERIMENTAL: May be changed or removed. 


@gedw99 gedw99 changed the title caddy add-package does not allow adding by version "caddy add-package" does not allow adding by version Aug 29, 2024
@gedw99 gedw99 changed the title "caddy add-package" does not allow adding by version "caddy add-package" does not allow adding by version or json out Aug 29, 2024
@gedw99
Copy link
Author

gedw99 commented Aug 29, 2024

Note sure if it's a bug but caddy list-modules --packages --versions returns double listing of each module.

Maybe it should match how the standard modules look, which is tidy and also importantly able to be parsed by the space delimiter, for basic automation:

tls.stek.standard v2.8.4 github.com/caddyserver/caddy/v2

Here is the current output:

caddy list-modules --packages --versions

...

tls.stek.standard v2.8.4 github.com/caddyserver/caddy/v2

  Standard modules: 121

exec v0.0.0-20240603212820-a42a5b2ae10f github.com/abiosoft/caddy-exec
http.handlers.exec v0.0.0-20240603212820-a42a5b2ae10f github.com/abiosoft/caddy-exec

  Non-standard modules: 2

  Unknown modules: 0

@gedw99 gedw99 changed the title "caddy add-package" does not allow adding by version or json out "caddy add-package" does not allow adding by version Aug 29, 2024
@gedw99 gedw99 changed the title "caddy add-package" does not allow adding by version "caddy add-package" could allow adding by version Aug 29, 2024
@francislavoie
Copy link
Member

What do you mean "double listing"? Nothing is doubled there. Do you mean exec? Those are in fact two separate modules.

@gedw99
Copy link
Author

gedw99 commented Aug 29, 2024

What do you mean "double listing"? Nothing is doubled there. Do you mean exec? Those are in fact two separate modules.

To me, it looks like it is the same module. but listed on 2 lines. Also the same version.

Relevant code is here ?

func cmdListModules(fl Flags) (int, error) {

@francislavoie
Copy link
Member

They are separate. One is a Caddy app, the other is an http handler directive. Both come from the same package of course.

@gedw99
Copy link
Author

gedw99 commented Aug 29, 2024

What do you mean "double listing"? Nothing is doubled there. Do you mean exec? Those are in fact two separate modules.

To me, it looks like it is the same module. but listed on 2 lines. Also the same version.

Relevant code is here ?

func cmdListModules(fl Flags) (int, error) {

So it’s 2 lines of the same package ? Did you decide what it is ?

They are separate. One is a Caddy app, the other is an http handler directive. Both come from the same package of course.

Either way . It seems confusing visually to me. Probably to others too .

What would you suggest we do ?

@francislavoie
Copy link
Member

francislavoie commented Aug 29, 2024

Yes it's these two modules:

There's nothing confusing here. It's working exactly as intended. One is an App, one is HTTP handler, like I said. See the README for that package to understand the difference. The App is for "global config" and the handler is for per-site config.

@mholt mholt closed this as not planned Won't fix, can't repro, duplicate, stale Sep 2, 2024
@mholt
Copy link
Member

mholt commented Sep 5, 2024

Reopening since we could still allow specifying versions in the command

@mholt mholt reopened this Sep 5, 2024
@gedw99
Copy link
Author

gedw99 commented Sep 8, 2024

Reopening since we could still allow specifying versions in the command

Hey @mholt

do you mean that your open to adding a version flag ?

@francislavoie
Copy link
Member

It should probably match the xcaddy CLI syntax I think, i.e. @<ref> suffix to the package name.

@mholt
Copy link
Member

mholt commented Sep 9, 2024

Beat me to it, but yeah I think symmetry with the xcaddy CLI would be a good idea. package@ref syntax.

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 a pull request may close this issue.

3 participants