Skip to content

Conversation

@akartsky
Copy link
Contributor

@akartsky akartsky commented Sep 20, 2022

Provide option for users to specify Role labels in helm values

This is useful when using k8s clusterRole aggregation
We can use aggregation rule labels to give other applications access to ack resources (Eg: Kubeflow in our case)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@akartsky akartsky changed the title New helm variable to provide clusterRole label option [helm] Add new helm variable to provide clusterRole label option Sep 20, 2022
@akartsky
Copy link
Contributor Author

/test s3-controller-test

@akartsky akartsky changed the title [helm] Add new helm variable to provide clusterRole label option [helm] Add new helm variable to provide clusterRole label Sep 20, 2022
@akartsky akartsky changed the title [helm] Add new helm variable to provide clusterRole label [helm] Add helm variable for ClusterRole Label Sep 20, 2022
@akartsky akartsky changed the title [helm] Add helm variable for ClusterRole Label [helm] Add helm variable for ClusterRole Labels Sep 20, 2022
Copy link
Member

@surajkota surajkota left a comment

Choose a reason for hiding this comment

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

Thanks

@surajkota
Copy link
Member

/retest

@vijtrip2
Copy link
Contributor

/test s3-controller-test

@akartsky
Copy link
Contributor Author

S3 test failing with

==== building s3-controller ====
Copying common custom resource definitions into s3
Building Kubernetes API objects for s3
Generating deepcopy code for s3
github.com/aws-controllers-k8s/s3-controller/apis/v1alpha1:-: invalid pointer element type: invalid type
Error: not all generators ran successfully
run `controller-gen object:headerFile=/home/prow/go/src/github.com/aws-controllers-k8s/code-generator/scripts/../templates/boilerplate.txt paths=./... -w` to see all available markers, or `controller-gen object:headerFile=/home/prow/go/src/github.com/aws-controllers-k8s/code-generator/scripts/../templates/boilerplate.txt paths=./... -h` for usage
make: *** [Makefile:41: build-controller] Error 1
generate-test-controller.sh][ERROR] Failed to generate new controller. Exiting ...

@akartsky akartsky force-pushed the helm_clusterrole_label branch from e177e8e to d42b34c Compare September 20, 2022 15:32
@RedbackThomson
Copy link
Contributor

/approve

@RedbackThomson RedbackThomson changed the title [helm] Add helm variable for ClusterRole Labels Add helm variable for ClusterRole Labels Sep 20, 2022
@akartsky akartsky changed the title Add helm variable for ClusterRole Labels Add helm variable for Role Labels Sep 20, 2022
@a-hilaly
Copy link
Member

@akartsky @surajkota the s3-controller test failures are not related to this PR

@akartsky akartsky force-pushed the helm_clusterrole_label branch from d42b34c to 2e0961a Compare September 20, 2022 17:07
@ack-bot
Copy link
Collaborator

ack-bot commented Sep 20, 2022

@akartsky: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
s3-controller-test 2e0961a link /test s3-controller-test
dynamodb-controller-test 2e0961a link /test dynamodb-controller-test

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@akartsky
Copy link
Contributor Author

/test dynamodb-controller-test

Copy link
Member

@a-hilaly a-hilaly left a comment

Choose a reason for hiding this comment

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

/lgtm

@ack-bot ack-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 20, 2022
@ack-bot
Copy link
Collaborator

ack-bot commented Sep 20, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: A-Hilaly, akartsky, RedbackThomson

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [A-Hilaly,RedbackThomson]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-bot ack-bot merged commit 6e2ffbc into aws-controllers-k8s:main Sep 20, 2022
Vandita2020 pushed a commit to Vandita2020/ack-code-generator that referenced this pull request Oct 24, 2022
Provide option for users to specify Role labels in helm values 

This is useful when using [k8s clusterRole aggregation](https://kubernetes.io/docs/reference/access-authn-authz/rbac/#aggregated-clusterroles)
We can use aggregation rule labels to give other applications access to ack resources (Eg: Kubeflow in our case)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants