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

store: Break apart Store interface #174

Open
adamdecaf opened this issue Mar 1, 2018 · 2 comments
Open

store: Break apart Store interface #174

adamdecaf opened this issue Mar 1, 2018 · 2 comments

Comments

@adamdecaf
Copy link
Owner

adamdecaf commented Mar 1, 2018

I'm starting to see store.Store grow beyond a desired size. First it was adding GetInfo(), which wasn't too bad. Afterwords it's becoming cmd/whitelist: fail if we don't have a backup and cmd/chain: Introduce a certificate chain viewer and cmd/create: output certificate chains, bundles, truststores, etc.

I'm proposing to breakup store.Store into:

type Saver interface { 
	Backup() error
	GetLatestBackup() (string, error) // https://github.com/adamdecaf/cert-manage/issues/53
	Restore(where string) error
}

type ListOptions struct { // https://github.com/adamdecaf/cert-manage/issues/172 
	Trusted bool
	Untrusted bool
}
type Lister interface {
	List(opts *ListOptions) ([]*x509.Certificate, error)
}
type Modifier interface {
	Add([]*x509.Certificate) error
	Remove(whitelist.Whitelist) error
}
type Store interface {
	Lister
	Modifier
	Saver
	
	GetInfo() *Info
}

We should probably split things out into subpackages too.. For a java.New(..) for example: #139

The rest can stay on Store and Store should be required to implement Saver and Lister. We just need to expand the options.

@adamdecaf
Copy link
Owner Author

I tried splitting each store into it's own package, but ran into cyclic deps.

$ make dist
go vet ./...
import cycle not allowed
package github.com/adamdecaf/cert-manage
	imports github.com/adamdecaf/cert-manage/pkg/cmd
	imports github.com/adamdecaf/cert-manage/pkg/store
	imports github.com/adamdecaf/cert-manage/pkg/store/java
	imports github.com/adamdecaf/cert-manage/pkg/store

@adamdecaf
Copy link
Owner Author

adamdecaf commented Mar 2, 2018

It's broken up, but we can probably just get rid of Store totally.

@adamdecaf adamdecaf removed this from the 0.2.0 milestone Mar 2, 2018
@adamdecaf adamdecaf removed their assignment Mar 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant