-
Notifications
You must be signed in to change notification settings - Fork 487
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
Introduce the gcp_cloudstorage
BundlePublisher plugin
#4961
Introduce the gcp_cloudstorage
BundlePublisher plugin
#4961
Conversation
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
storage.objects.delete | ||
``` | ||
|
||
The `storage.objects.delete` permission is required to overwrite the object when the bundle is updated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to update? instead of deleting? or that is the way gcp api works?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is how the Google Cloud Storage API works, as documented here: https://cloud.google.com/storage/docs/uploading-objects#roles-and-permissions
storage.objects.delete
This permission is only required for uploads that overwrite an existing object.
|
||
## Sample configuration | ||
|
||
The following configuration uploads the local trust bundle contents to the `example.org` object in the `spire-trust-bundle` bucket. The AWS access key id and secret access key are obtained from the AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESSKEY environment variables. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this example is calling spire-bundle
can you unify?
|
||
The `storage.objects.delete` permission is required to overwrite the object when the bundle is updated. | ||
|
||
## Sample configuration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: old plugin used to have an example using service_account_file, what do you think about adding the same here?
doc/spire_server.md
Outdated
@@ -41,7 +41,8 @@ This document is a configuration reference for SPIRE Server. It includes informa | |||
| UpstreamAuthority | [cert-manager](/doc/plugin_server_upstreamauthority_cert_manager.md) | Uses a referenced cert-manager Issuer to request intermediate signing certificates. | | |||
| Notifier | [gcs_bundle](/doc/plugin_server_notifier_gcs_bundle.md) | A notifier that pushes the latest trust bundle contents into an object in Google Cloud Storage. | | |||
| Notifier | [k8sbundle](/doc/plugin_server_notifier_k8sbundle.md) | A notifier that pushes the latest trust bundle contents into a Kubernetes ConfigMap. | | |||
| BundlePublisher | [aws_s3](/doc/plugin_server_bundlepublisher_aws_s3.md) | Publishes trust bundles to an Amazon S3 bucket. | | |||
| BundlePublisher | [aws_s3](/doc/plugin_server_bundlepublisher_aws_s3.md) | Publishes the trust bundle to an Amazon S3 bucket. | | |||
| BundlePublisher | [gcp_cloudstorage](/doc/plugin_server_bundlepublisher_gcp_cloudstorage.md) | Publishes the trust bundle to a Google Cloud Storage bucket. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you resolve identation issues in this table?
// from storageWriter.Close(). | ||
if err != nil { | ||
// Close the storage writer before returning. | ||
if closeErr := storageWriter.Close(); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why no to use a defer after writer was created?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the _, err = storageWriter.Write(bundleBytes)
call returns an error, we want to first call Close() and then return the error returned by Write, so I don't think that using defer
would help here.
} | ||
|
||
p.setBundle(req.Bundle) | ||
p.log.Debug("Bundle published") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there some metadata that can be added as field here? that simplify debugging?
) | ||
|
||
var ( | ||
writeObjectCount int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since packages are shared (code with test) can you rename this var to something like testWriteObjectCount
?
{ | ||
name: "success", | ||
bundle: testBundle, | ||
config: &Config{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since config is pretty much the same except test that validates that plugin was never configured,
what do you think about adding a bool to tt, something like noConfigure bool
and just no run configure when that is true, and make this config always default option?
} | ||
newStorageWriter := getFakeNewStorageWriterFunc(nil, nil) | ||
p := newPlugin(newClient, newStorageWriter) | ||
p.hooks.wroteObjectFunc = func() { writeObjectCount++ } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like this var wroteObjectCount
is only used in this function, may we create it here?
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
* Introduce the `gcp_cloudstorage` BundlePublisher plugin Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com> * Address PR comments Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com> --------- Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com> Co-authored-by: Marcos Yacob <marcos.yacob@hpe.com>
This PR introduces the
gcp_cloudstorage
BundlePublisher plugin, that puts the current trust bundle of the server in a designated Google Cloud Storage bucket, keeping it updated.