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

feat: add value for extending minio policies #68

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Diddy42
Copy link

@Diddy42 Diddy42 commented Dec 14, 2023

This PR adds the value deploykf_opt.deploykf_minio.extraPolicyStatements, which makes it possible to add custom MinIO policy statements to specific users or groups. In case of groups, there are added to all users of that group.

For example:

extraPolicyStatements:

      ## additional policy statements for 'user-1' 
      - user: user-1
        statements:
          ## allow 'user-1' to see MY_BUCKET_NAME
          - Effect: Allow
            Action:
              - s3:GetBucketLocation
              - s3:ListBucket
            Resource:
              - arn:aws:s3:::MY_BUCKET_NAME

      ## additional policy statements for each member of 'team-1' group
      - group: team-1
        statements:
          ## allow members of 'team-1' to see MY_BUCKET_NAME
          - Effect: Allow
            Action:
              - s3:GetBucketLocation
              - s3:ListBucket
            Resource:
              - arn:aws:s3:::MY_BUCKET_NAME
          
          ## allow members of 'team-1' to read/write under MY_BUCKET_NAME/some-prefix/*
          - Effect: Allow
            Action:
              - s3:GetObject
              - s3:PutObject
              - s3:DeleteObject
            Resource:
              - arn:aws:s3:::MY_BUCKET_NAME/some-prefix/*

For context, this feature stems from this discussion.

Implementation comments

  • Instead of modifying kubeflow_pipelines.object_store.user.minio_policy to allow for extra statements, I opted for creating a new one (generator/helpers/deploykf-minio-policies.tpl), as this functionality is not really relevant to the kubeflow_pipelines's object_store. Hence, it seems cleaner this way (unless I'm missing something, I'm interested to know your thoughts!). This helper file is used to append policy statements to an existing policy.
    • This helper file also hardcodes the Version field to "2012-10-17", as kubeflow_pipelines.object_store.user.minio_policy does. It would be cleaner to take the Version from the existing policy (with Version: {{< $prev_policy.Version >}}), but the value gets converted to 2012-10-17Z00:00:00 and I haven't found a way around that. Since it is also hardcoded in other places and is not settable by the user, I don't think it is a big deal, but I am open to feedback.
  • Open to any suggestions/improvements on code style or design, as this would be my first contribution here!

closes: #69

Signed-off-by: Nadir Casciola <nadir.casciola@gmail.com>
@thesuperzapper thesuperzapper added status/do-not-merge status - something is blocking this status/needs-discussion status - this needs discussion and removed status/do-not-merge status - something is blocking this labels Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/feature status/needs-discussion status - this needs discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add extraPolicyStatements for embedded MinIO (define user access)
2 participants