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

RemoveContainer for local seems incorrect #239

Open
acabarbaye opened this issue Jul 3, 2020 · 0 comments
Open

RemoveContainer for local seems incorrect #239

acabarbaye opened this issue Jul 3, 2020 · 0 comments

Comments

@acabarbaye
Copy link

acabarbaye commented Jul 3, 2020

https://github.com/graymeta/stow/blob/cacc23c0285416ac720e9f73cbe150d22c84bc2d/local/location.go#L30

I think the implementation should be similar to CreateContainer(name)

	path, ok := l.config.Config(ConfigKeyPath)
	if !ok {
		return nil, errors.New("missing " + ConfigKeyPath + " configuration")
	}
	fullpath := filepath.Join(path, name)
        return os.RemoveAll(fullpath)
jlucktay added a commit to TykTechnologies/mserv that referenced this issue Mar 16, 2021
The RemoveContainer implementation for "local" Kind in stow does not use the full path when
deleting, only a relative one. If WasabiAiR/stow#239 were to be
implemented/resolved then this block of code could be deleted.
jlucktay added a commit to TykTechnologies/mserv that referenced this issue Mar 17, 2021
* refactor(http_funcs): carve up test files ahead of adding coverage
* feat(configuration): add capability to refresh and reload config from disk
Resetting the configuration for mserv to zero and (re)reading afresh aids testability.

* ci(golangci-lint): remove as per warning; maligned has since moved into 'go vet'
* test(http_funcs): add coverage for DeleteMW handler
Added a new test, along with some helpers and mock implementations, to cover the DeleteMW. Note that
the test is currently failing as of this commit, so let's see about fixing that!

* refactor(api): set up format string const for container name pattern
* refactor(api): use package consts instead of hard-coding Kind strings
* fix(api): also remove stow container when deleting a bundle
There was previously no call made from the bundle delete handler for stow to remove its local/S3
container, so local files and S3 buckets would be left lying around after their corresponding
middleware definitions within MServ wer long gone.

fix TT-1802

* fix(api): work around shortcoming with stow's "local" Kind
The RemoveContainer implementation for "local" Kind in stow does not use the full path when
deleting, only a relative one. If WasabiAiR/stow#239 were to be
implemented/resolved then this block of code could be deleted.

* fix(api): clean up lint issue by catching/logging potential error from Close call
* fix(api): walk container and remove each item before removing container, for S3's sake
Via Stow, S3 buckets can only be removed once they no longer contain any objects.

fix TT-1802

* refactor(api): fix lint issues; stop shadowing error declarations
* refactor(api): move store.DeleteMW call to end of handler rather than beginning
We still do a GetMWByID call on the middleware in the store at the start of the handler, which would
error and return immediately if middleware with that ID did not exist. Given that succeeds though,
we proceed to clean out the stow container and remove that, which is much more error-prone given the
new changes in this feature branch. Assuming all of that succeeds, then we delete the middleware
from the store.

* feat(api): export format string for container names
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

No branches or pull requests

1 participant