Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

fleetctl using wildcards can be dangerous #710

Closed
developerinlondon opened this issue Jul 25, 2014 · 6 comments
Closed

fleetctl using wildcards can be dangerous #710

developerinlondon opened this issue Jul 25, 2014 · 6 comments
Labels
Milestone

Comments

@developerinlondon
Copy link

currently using wildcards with fleetctl destroy x picks up the filenames from the current directory instead of whats shown in fleetctl list-units

this can be dangerous as the current directory does not necessarily reflect whats in list-units.

the correct behaviour might be to use the wildcards for units that are already there or not accept wildcards at all.

@bcwaldon bcwaldon added the bug label Jul 30, 2014
@wuqixuan
Copy link
Contributor

wuqixuan commented May 8, 2015

@bcwaldon , I checked the code, in main() function, os.Exit(cmd.Run(cmd.Flags.Args())), cmd.Flags.Args() function call will find the file using wildcards in current directory. I think should not give the array of units to cmd.Run. Need search from etcd/http client , then pass it to cmd.Run.
What do you think? Should we modify it ?

@wuqixuan
Copy link
Contributor

wuqixuan commented May 8, 2015

@bcwaldon , If need modify as what I suggest, I can help to implement.

wuqixuan pushed a commit to wuqixuan/fleet that referenced this issue Jun 18, 2015
Curerntly, fleetctl destroy *.service, it will search the units
in the local directory. The args with wildcards have already parsed
by golang package before be passed into runDestroyUnits. So
runDestroyUnits cannot do anything.

This patch support wildcards just search in the repository,
not from local directory. But need support a new interface. The new
interface is:
    fleetctl destroy "xx*.service".
We found that if bracket the service name, the arg will not be
parsed by golang package to local directory. Then we can do match it
with the service name in the repository.

Fixes coreos#710
wuqixuan pushed a commit to wuqixuan/fleet that referenced this issue Jun 18, 2015
Curerntly, fleetctl destroy *.service, it will search the units
in the local directory. The args with wildcards have already parsed
by golang package before be passed into runDestroyUnits. So
runDestroyUnits cannot do anything.

This patch support wildcards just search in the repository,
not from local directory. But need support a new interface. The new
interface is:
    fleetctl destroy "xx*.service".
We found that if bracket the service name, the arg will not be
parsed by golang package to local directory. Then we can do match it
with the service name in the repository.

Fixes coreos#710
@wuqixuan
Copy link
Contributor

@bcwaldon @jonboulle , I have given the patch. Do you have any comment ?

wuqixuan pushed a commit to wuqixuan/fleet that referenced this issue Jun 18, 2015
Curerntly, fleetctl destroy *.service, it will search the units
in the local directory. The args with wildcards have already parsed
by golang package before be passed into runDestroyUnits. So
runDestroyUnits cannot do anything.

This patch support wildcards just search in the repository,
not from local directory. But need support a new interface. The new
interface is:
    fleetctl destroy "xx*.service".
We found that if bracket the service name, the arg will not be
parsed by golang package to local directory. Then we can do match it
with the service name in the repository.

Fixes coreos#710
@jonboulle jonboulle added kind/bug and removed bug labels Sep 24, 2015
@jonboulle jonboulle added this to the v0.13.0 milestone Jan 25, 2016
@jonboulle
Copy link
Contributor

Let's try shepherd this change in for v0.13.0

@antrik
Copy link
Contributor

antrik commented Mar 14, 2016

This is not entirely clear: but it seems like @developerinlondon assumed the undesired file globbing was performed by fleetctl itself (hello Windows...) -- whereas it's in fact performed by the shell. As such, I guess this bug qualifies as invalid.

The linked PR on the other hand proposes adding globbing of units within fleetctl -- which is an entirely new feature; and is only marginally related to the actual problem at hand IMHO... Having said that, it sounds fairly useful on it's own -- but it needs quite some work. (See my comment over there.)

@jonboulle
Copy link
Contributor

@antrik I agree with your assessment. Let's continue in #1256.

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants