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

added interface for collector #5

Closed
wants to merge 3 commits into from
Closed

Conversation

pxp928
Copy link
Collaborator

@pxp928 pxp928 commented Aug 15, 2022

Signed-off-by: pxp928 parth.psu@gmail.com

Added interface for collector. Utilizing a configuration to specify the bucket address and such. Currently implementing the google cloud bucket interface.

@pxp928 pxp928 force-pushed the bucket_ingest branch 6 times, most recently from 9b3c141 to 946f3bf Compare August 15, 2022 21:24
pkg/config/config.go Outdated Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
pkg/ingestor/collector/collector.go Outdated Show resolved Hide resolved
pkg/ingestor/collector/gcs/gcs.go Show resolved Hide resolved
pkg/ingestor/collector/gcs/gcs.go Outdated Show resolved Hide resolved
@pxp928
Copy link
Collaborator Author

pxp928 commented Aug 17, 2022

@lumjjb Ready for re-review.

pkg/config/config.go Outdated Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
pkg/ingestor/collector/collector.go Show resolved Hide resolved
pkg/ingestor/collector/collector.go Outdated Show resolved Hide resolved
pkg/ingestor/collector/transparency/transparency.go Outdated Show resolved Hide resolved
want *Backend
wantErr bool
}{
// TODO: Add test cases.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's add some here :)

Signed-off-by: pxp928 <parth.psu@gmail.com>
@pxp928 pxp928 force-pushed the bucket_ingest branch 4 times, most recently from cdc3fc5 to b0e4ec1 Compare August 21, 2022 01:07
Signed-off-by: pxp928 <parth.psu@gmail.com>
Signed-off-by: pxp928 <parth.psu@gmail.com>
Copy link
Collaborator

@mihaimaruseac mihaimaruseac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(looked at it thinking it was the PR we paired on today, got a few questions only)

Comment on lines +18 to +48
type Config struct {
Collector CollectorConfigs
}

// CollectorConfigs contains the configuration to instantiate different collector providers
type CollectorConfigs struct {
GCS GCSSCollectorConfig
OCI OCICollectorConfig
Transparency TransparencyConfig
PubSub PubSubCollectorConfig
}

type GCSSCollectorConfig struct {
Bucket string
}

type OCICollectorConfig struct {
Repository string
Insecure bool
}

type PubSubCollectorConfig struct {
Provider string
Topic string
}

type TransparencyConfig struct {
Enabled bool
VerifyAnnotation bool
URL string
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking that we can do a modular registration approach like in

var (
documentProcessors = map[processor.DocumentType]processor.DocumentProcessor{}
)
func init() {
// Registerprocessor.DocumentProcessor()
}
func RegisterDocumentProcessor(p processor.DocumentProcessor, d processor.DocumentType) {
if _, ok := documentProcessors[d]; ok {
logrus.Warnf("the document processor is being overwritten: %s", d)
}
documentProcessors[d] = p
}
here too. This way, when we add a new collector config, we only need to import the package that defines the global registry, let its init initialize whatever additional static fields are needed, call the Register... function and later at runtime select the corresponding collector from the map.

We can also go with this for now and refactor later, though it might be more work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this was the piece I was asking about. Need to rethink the initializing piece. I will do it the other way as it makes more sense.

Comment on lines +24 to +27
GCS GCSSCollectorConfig
OCI OCICollectorConfig
Transparency TransparencyConfig
PubSub PubSubCollectorConfig
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is like a sum type, right? That is, only one of these configs should be set at any given time during runtime?

@pxp928
Copy link
Collaborator Author

pxp928 commented Aug 23, 2022

Closing PR for redesign.

@pxp928 pxp928 closed this Aug 23, 2022
mrizzi pushed a commit to mrizzi/guac that referenced this pull request Aug 25, 2023
Add CertifyVEXStatement and IngestVEXStatement
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 this pull request may close these issues.

None yet

3 participants