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

Create breakdown for additional Storage examples. #2256

Closed
17 tasks done
coryan opened this issue Mar 19, 2019 · 14 comments
Closed
17 tasks done

Create breakdown for additional Storage examples. #2256

coryan opened this issue Mar 19, 2019 · 14 comments
Assignees
Labels
api: storage Issues related to the Cloud Storage API. type: docs Improvement to the documentation for an API.

Comments

@coryan
Copy link
Contributor

coryan commented Mar 19, 2019

For each region tag (and description) below we need to either:

  • Check if the example exists, if it does, create a bug (or simply a PR) to fix the region tag. I think there are several in that category, I am blinding copying the region tag descriptions here.
  • If the example does not exist, create a bug to implement that example. If multiple examples can be grouped in a single bug that is Okay.

We can close this bug once all the child bugs are created.

  • storage_create_bucket_class_location: Create a GCS bucket with storage class and location parameters
  • storage_add_bucket_owner: Add owner access control to a GCS bucket
  • storage_remove_bucket_owner: Remove owner access control to a GCS bucket
  • storage_add_bucket_default_owner: Add default access control to a GCS bucket
  • storage_add_file_owner: Add owner access control to an object in a GCS bucket
  • storage_remove_file_owner: Remove owner access to an object in a GCS bucket
  • storage_print_file_acl_for_user: View the access control list for an object in a GCS bucket, filtered by user
  • storage_change_file_storage_class: Rewrite and update the object storage class of a GCS object
  • storage_view_lifecycle_management_configuration: View lifecycle configuration of a GCS bucket
  • storage_define_bucket_website_configuration: Set static website configuration for a GCS bucket
  • storage_enable_versioning: Enable object versioning for a GCS bucket
  • storage_disable_versioning: Disable object versioning for a GCS bucket
  • storage_view_versioning_status: Check if object versioning status is enabled for a GCS bucket
  • storage_copy_file_archived_generation: Copy an archived generation of an object in a GCS bucket
  • storage_delete_file_archived_generation: Delete an archived generation of an object in a GCS bucket
  • storage_download_public_file: Download an object from a public data set object
  • storage_cors_configuration: Add CORS configuration to a GCS storage bucket
@coryan coryan added the api: storage Issues related to the Cloud Storage API. label Mar 19, 2019
@coryan coryan added this to the Cloud Storage: GA milestone Mar 19, 2019
coryan added a commit to coryan/google-cloud-cpp that referenced this issue Mar 19, 2019
I copied the wrong region tags into the previous set of bugs.

See googleapis#2256 for details.
@coryan
Copy link
Contributor Author

coryan commented Mar 19, 2019

I just submitted a PR to fix several region tags. I clicked their checkboxes already.

@coryan coryan added the type: docs Improvement to the documentation for an API. label Mar 19, 2019
coryan added a commit that referenced this issue Mar 19, 2019
I copied the wrong region tags into the previous set of bugs.

See #2256 for details.
@ghost
Copy link

ghost commented Mar 19, 2019

What should the region tags be for disabling or viewing the bucket website configuration?

@coryan
Copy link
Contributor Author

coryan commented Mar 20, 2019

What should the region tags be for disabling or viewing the bucket website configuration?

@remyabel I looked again, and there is not one defined. If we add such an example we can use storage_print_website_configuration (there are 5 storage_print_* region tags and 3 storage_view_* region tags, so I am going with print).

/cc: @frankyn FYI in case you have an opinion.

@frankyn
Copy link
Member

frankyn commented Mar 20, 2019

print is fine. The region tags should be updated to be more consistent. I'll create a new sample request with this region tag to track this internally.

Thanks @remyabel!

@ghost
Copy link

ghost commented Mar 22, 2019

What would be the difference between storage_delete_bucket_acl and storage_remove_bucket_owner? This works:

    StatusOr<gcs::BucketAccessControl> patched_acl =
        client.PatchBucketAcl(bucket_name, entity,
                              gcs::BucketAccessControlPatchBuilder().set_role(
                                  gcs::BucketAccessControl::ROLE_OWNER()));

This does not:

    StatusOr<gcs::BucketAccessControl> patched_acl = client.PatchBucketAcl(
        bucket_name, entity,
        gcs::BucketAccessControlPatchBuilder().delete_role());

It errors with type must be string, but is null even though the docs say to delete a field you set it to NULL.

@ghost
Copy link

ghost commented Mar 23, 2019

So turns out that error is from nlohmann json. This:

StatusOr<gcs::BucketAccessControl> patched_acl = client.PatchBucketAcl(bucket_name, entity,
                      gcs::BucketAccessControlPatchBuilder().set_role(
                          gcs::BucketAccessControl::ROLE_OWNER()).delete_entity());

seems very close to the example from bucket_acl_requests_test.cc:

  PatchBucketAclRequest request(
      "my-bucket", "user-test-user",
      BucketAccessControlPatchBuilder().set_role("READER").delete_entity());
  nl::json expected = {{"role", "READER"}, {"entity", nullptr}};

Unless I'm missing something obvious. This compiles and the tests pass now, but it doesn't work in practice:

root@8e5bb9d8bec3:/v# env GOOGLE_APPLICATION_CREDENTIALS=$PWD/credentials ./build-output/ubuntu-18.04-clang-Debug/google/cloud/storage/examples/storage_bucket_acl_samples remove-bucket-owner asdfasdf12345 allAuthenticatedUsers
Standard C++ exception raised: Permanent error in PatchBucketAcl: {
 "error": {
  "errors": [
   {
    "domain": "global",
    "reason": "invalidParameter",
    "message": "Value 'null' in content does not agree with value 'allAuthenticatedUsers'. This can happen when a value set through a parameter is inconsistent with a value set in the request."
   }
  ],
  "code": 400,
  "message": "Value 'null' in content does not agree with value 'allAuthenticatedUsers'. This can happen when a value set through a parameter is inconsistent with a value set in the request."
 }
}
 [INVALID_ARGUMENT]

@coryan
Copy link
Contributor Author

coryan commented Mar 23, 2019

What would be the difference between storage_delete_bucket_acl and storage_remove_bucket_owner?

The second should probably remove a entity from the access control list that has OWNER privileges (as opposed to something milder).

It errors with type must be string, but is null even though the docs say to delete a field you set it to NULL.

That documentation is less than ideal. To remove a field you set it to null, but to remove an entry from a list you provide a new list with the entry removed 😞

So what you want to do is get the bucket ACL, remove the entry you do not want, and patch the bucket with the new ACL.

@ghost
Copy link

ghost commented Mar 23, 2019

Thanks. One more question (sorry if this is annoying), GetBucketAcl returns a single ACL for a particular entity, right? So if we want to remove an entity from an ACL, I use acl() from BucketMetadata instead?

@coryan
Copy link
Contributor Author

coryan commented Mar 23, 2019

Oh, now that you mention that, there is this:

Status DeleteBucketAcl(std::string const& bucket_name,

That seems like the right API to document here. There is an existing example, we may be able to just re-use that example for this.

Yes, there are two ways to manipulate the ACLs in a bucket: the {Create,Delete,Get,List,Update,Patch}BucketAcl functions, through the metadata functions. Actually it is three, you can do IAM policies too.

I think we should use DeleteBucketAcl here.

@ghost
Copy link

ghost commented Mar 23, 2019

If we just use DeleteBucketAcl(bucket_name, entity) by itself, doesn't storage_remove_bucket_owner become redundant? The .NET samples do this:

bucket.Acl = bucket.Acl.Where((acl) =>
    !(acl.Entity == $"user-{userEmail}" && acl.Role == "OWNER")
).ToList();

My similar attempt:

std::vector<gcs::BucketAccessControl> original_acl =
        original_metadata->acl();
    auto it = std::find_if(original_acl.begin(), original_acl.end(),
                           [entity](const gcs::BucketAccessControl& entry) {
                             return entry.entity() == entity &&
                                    entry.role() ==
                                        gcs::BucketAccessControl::ROLE_OWNER();
                           });

The problem is original_metadata->acl() seems to be empty. I guess I'm missing how you can remove an entry from ACLs using the BucketAcl API, but I'll take a break from this and let someone else tackle it.

@coryan
Copy link
Contributor Author

coryan commented Mar 23, 2019

The problem is original_metadata->acl() seems to be empty.

Did you use gcs::Projection::Full():

static Projection Full() { return Projection("full"); }

The default (i.e. if we do not send a projection to the server) is equivalent to NoAcl().

I guess I'm missing how you can remove an entry from ACLs using the BucketAcl API, but I'll take a break from this and let someone else tackle it.

Fair enough.

@ghost
Copy link

ghost commented Mar 24, 2019

I believe these already have implementations:

storage_add_bucket_default_owner

void CreateDefaultObjectAcl(google::cloud::storage::Client client, int& argc,

storage_print_file_acl_for_user

https://github.com/googleapis/google-cloud-cpp/blob/master/google/cloud/storage/examples/storage_object_acl_samples.cc#L130

@ghost
Copy link

ghost commented Mar 26, 2019

I think this can be closed now. All samples should either be implemented or have open issues.

@coryan
Copy link
Contributor Author

coryan commented Mar 26, 2019

Agree, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the Cloud Storage API. type: docs Improvement to the documentation for an API.
Projects
None yet
Development

No branches or pull requests

2 participants