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

[Feature] Consistency check for RBAC #577

Merged
merged 13 commits into from
Sep 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 44 additions & 6 deletions .github/workflows/consistency-check.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: CRD-consistency-check
name: operator-consistency-check

on:
push:
Expand All @@ -16,63 +16,101 @@ jobs:
- uses: actions/checkout@v2
with:
fetch-depth: 0

- name: Set up Go 1.17.x
uses: actions/setup-go@v2
with:
# Use the same go version with build job
go-version: '1.17'

- name: Check golang version
working-directory: ./ray-operator
run: go version

- name: Verify Codegen
working-directory: ./ray-operator
run: ./hack/verify-codegen.sh
# Check consistency between types.go and CRD YAML files (manifest)
ray-operator-verify-crd:
# 1. Check consistency between types.go and CRD YAML files.
# 2. Check consistency between kubebuilder markers and RBAC.
ray-operator-verify-crd-rbac:
runs-on: ubuntu-latest
timeout-minutes: 10
steps:
- uses: actions/checkout@v2
with:
fetch-depth: 0
- name: Update CRD YAML files

- name: Update CRD/RBAC YAML files
working-directory: ./ray-operator
run: make manifests

- name: Verify Changed files
id: verify-changed-files
uses: tj-actions/[email protected]
with:
files: |
./config/bases/*.yaml
./config/rbac/*.yaml

- name: Check changed files
if: steps.verify-changed-files.outputs.files_changed == 'true'
uses: actions/github-script@v3
with:
script: |
core.setFailed('Please run \"make manifests\" to synchronize CRD!')
core.setFailed('Please run \"make manifests\" to synchronize CRD and RBAC!')
# Check consistency between CRD YAML files in ray-operator/config/crd/bases
# and in helm-chart/kuberay-operator/crds
helm-chart-verify-crd:
needs: ray-operator-verify-crd
needs: ray-operator-verify-crd-rbac
runs-on: ubuntu-latest
timeout-minutes: 10
steps:
- uses: actions/checkout@v2
with:
fetch-depth: 0

- name: Update CRD YAML files
run: |
rm -r helm-chart/kuberay-operator/crds/
cp -r ray-operator/config/crd/bases/ helm-chart/kuberay-operator/crds/

- name: Verify Changed files
id: verify-changed-files
uses: tj-actions/[email protected]
with:
files: |
./helm-chart/kuberay-operator/crds/*.yaml

- name: Check changed files
if: steps.verify-changed-files.outputs.files_changed == 'true'
uses: actions/github-script@v3
with:
script: |
core.setFailed('Please run \"make sync\" to synchronize CRDs!')
# Check consistency between RBAC YAML files in ray-operator/config/rbac/
# and in helm-chart/kuberay-operator/templates
helm-chart-verify-rbac:
needs: ray-operator-verify-crd-rbac
runs-on: ubuntu-latest
timeout-minutes: 10
steps:
- uses: actions/checkout@v2
with:
fetch-depth: 0

- name: Set up Helm
uses: azure/[email protected]
with:
version: v3.9.4

- uses: actions/setup-python@v4
with:
python-version: 3.7

- name: Install dependencies
run: |
pip3 install -r scripts/requirements.txt

- name: Check rbac consistency
run: |
python3 scripts/rbac-check.py
16 changes: 11 additions & 5 deletions ray-operator/DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,17 +97,23 @@ We have several [consistency checks](https://github.com/ray-project/kuberay/blob
1. `ray-operator/apis/ray/v1alpha1/*_types.go` should be synchronized with the CRD YAML files (`ray-operator/config/crd/bases/`)
2. `ray-operator/apis/ray/v1alpha1/*_types.go` should be synchronized with generated API (`ray-operator/pkg/client`)
3. CRD YAML files in `ray-operator/config/crd/bases/` and `helm-chart/kuberay-operator/crds/` should be the same.
4. Kubebuilder markers in `ray-operator/controllers/ray/*_controller.go` should be synchronized with RBAC YAML files in `ray-operator/config/rbac`.
5. RBAC YAML files in `helm-chart/kuberay-operator/templates` and `ray-operator/config/rbac` should be synchronized.

```bash
# Consistency 1:
# Synchronize consistency 1 and 4:
make manifests

# Consistency 2:
# Synchronize consistency 2:
./hack/update-codegen.sh

# Consistency 3:
# Synchronize consistency 3:
make helm

# Synchronize 1, 2, 3 in one command
# Synchronize 1, 2, 3, and 4 in one command
# [Note]: Currently, we need to synchronize consistency 5 manually.
make sync
```

# Reproduce CI error for job "helm-chart-verify-rbac" (consistency 5)
python3 ../scripts/rbac-check.py
```
1 change: 1 addition & 0 deletions ray-operator/controllers/ray/raycluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ type RayClusterReconciler struct {
// +kubebuilder:rbac:groups=core,resources=serviceaccounts,verbs=get;list;watch;create;delete
// +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=roles,verbs=get;list;watch;create;delete;update
// +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=rolebindings,verbs=get;list;watch;create;delete

// Reconcile used to bridge the desired state with the current state
func (r *RayClusterReconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) {
var err error
Expand Down
15 changes: 15 additions & 0 deletions scripts/helm-render-yaml.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/usr/bin/env bash
if [ -L ${BASH_SOURCE-$0} ]; then
PWD=$(dirname $(readlink "${BASH_SOURCE-$0}"))
else
PWD=$(dirname ${BASH_SOURCE-$0})
fi
export CURRENT_PATH=$(cd "${PWD}">/dev/null; pwd)
export KUBERAY_HOME=${CURRENT_PATH}/..

cd $KUBERAY_HOME/helm-chart/kuberay-operator/
declare -a YAML_ARRAY=("role.yaml" "ray_rayjob_editor_role.yaml" "ray_rayjob_viewer_role.yaml" "leader_election_role.yaml" "ray_rayservice_editor_role.yaml" "ray_rayservice_viewer_role.yaml" )
mkdir -p $KUBERAY_HOME/scripts/tmp
for name in "${YAML_ARRAY[@]}"; do
helm template -s templates/$name . > $CURRENT_PATH/tmp/$name
done
28 changes: 28 additions & 0 deletions scripts/rbac-check.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import os
import sys
from yaml import load, CLoader as Loader
from deepdiff import DeepDiff

def compare_two_yaml(yaml1, yaml2):
diff = DeepDiff(yaml1['rules'], yaml2['rules'])
if diff:
print(diff)
return not diff

if __name__ == "__main__":
curr_dir_path = os.path.dirname(os.path.realpath(__file__))
helm_rbac_dir = curr_dir_path + '/tmp/'
kustomize_rbac_dir = curr_dir_path + '/../ray-operator/config/rbac/'

os.system(f"{curr_dir_path}/helm-render-yaml.sh")
files = os.listdir(helm_rbac_dir)
diff_files = []

for f in files:
yaml1 = load(open(helm_rbac_dir + f, 'r'), Loader=Loader)
yaml2 = load(open(kustomize_rbac_dir + f, 'r'), Loader=Loader)
if not compare_two_yaml(yaml1, yaml2):
diff_files.append(f)

if diff_files:
sys.exit(f"{diff_files} are out of synchronization!")
2 changes: 2 additions & 0 deletions scripts/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
PyYAML==6.0
deepdiff==5.8.1