Task/csi 3417 add full generation for roles editing #220
Conversation
Signed-off-by: matancarmeli7 <[email protected]>
Signed-off-by: matancarmeli7 <[email protected]>
Signed-off-by: matancarmeli7 <[email protected]>
Signed-off-by: matancarmeli7 <[email protected]>
Signed-off-by: matancarmeli7 <[email protected]>
Signed-off-by: matancarmeli7 <[email protected]>
Signed-off-by: matancarmeli7 <[email protected]>
Signed-off-by: matancarmeli7 <[email protected]>
Signed-off-by: matancarmeli7 <[email protected]>
Signed-off-by: matancarmeli7 <[email protected]>
| @@ -3,1088 +3,1089 @@ kind: CustomResourceDefinition | |||
| metadata: | |||
| annotations: | |||
| controller-gen.kubebuilder.io/version: v0.4.1 | |||
| creationTimestamp: null | |||
There was a problem hiding this comment.
I think it relates that I changed the labels generation I guess, I don't think that it has any impact
There was a problem hiding this comment.
you mean because it was merged with kustomize and now it is merged with yq?
There was a problem hiding this comment.
how does yq even know about the creationTimestamp field?
yq is not related to k8s, unlike operator-sdk/kustomize
There was a problem hiding this comment.
because yq doesn't remove it and I think customize does
There was a problem hiding this comment.
then sounds like customize is better
Signed-off-by: matancarmeli7 <[email protected]>
Signed-off-by: matancarmeli7 <[email protected]>
hack/update-roles-in-csv.sh
Outdated
| # | ||
|
|
||
| get_latest_csi_version (){ | ||
| latest_csi_version=$(cat version/version.go | grep -i driverversion | awk -F = '{print $2}') |
There was a problem hiding this comment.
potential code dup detected with #157
please put it in a common place in both PRs, so we could at least keep track of the duplications using file and dir paths
There was a problem hiding this comment.
not sure I follow, but ok
hack/update-roles-in-csv.sh
Outdated
| "ibm-block-csi-operator-community" | ||
| "ibm-block-csi-operator" |
There was a problem hiding this comment.
we already maintain the bundle names, please move them to a common place
There was a problem hiding this comment.
I didn't mean that wildcard is better. I meant that we should maintain explicit "consts"/list of the bundle names/paths in one common place
Signed-off-by: matancarmeli7 <[email protected]>
Signed-off-by: matancarmeli7 <[email protected]>
Signed-off-by: matancarmeli7 <[email protected]>
Signed-off-by: matancarmeli7 <[email protected]>
| @@ -16,7 +16,7 @@ metadata: | |||
| description: "Run IBM block storage CSI driver." | |||
| repository: https://github.com/IBM/ibm-block-csi-operator | |||
| support: IBM | |||
| alm-examples: >- | |||
| alm-examples: |- | |||
There was a problem hiding this comment.
was this file changed manually or automatically?
There was a problem hiding this comment.
this change is manually
| align_crds | ||
| } | ||
|
|
||
| if [[ "${0##*/}" == "update-yamls-with-the-same-content.sh" ]]; then |
There was a problem hiding this comment.
(any chance to make it English?)
| yaml_file=$1 | ||
| required_lables=$2 |
There was a problem hiding this comment.
| yaml_file=$1 | |
| required_lables=$2 | |
| target_yaml_path=$1 | |
| lables_yaml_path=$2 |
| ["config/rbac/role.yaml"]="config/rbac/patches/role_labels_patch.yaml" | ||
| ["config/crd/bases/csi.ibm.com_ibmblockcsis.yaml"]="config/crd/patches/labels_patch.yaml" | ||
| ) | ||
| merge_yamls (){ |
There was a problem hiding this comment.
| merge_yamls (){ | |
| merge_yamls_into_first (){ |
?
| get_current_csi_version (){ | ||
| current_csi_version=$(cat version/version.go | grep -i driverversion | awk -F = '{print $2}') | ||
| echo ${current_csi_version//\"} | ||
| } |
There was a problem hiding this comment.
isn't this a common function with other files?
| @@ -0,0 +1,67 @@ | |||
| #!/bin/bash -e | |||
There was a problem hiding this comment.
update-yamls-with-the-same-content.sh is long. how about:
sync-common-yaml-contents.sh
| echo ${current_csi_version//\"} | ||
| } | ||
|
|
||
| are_csv_files_exsists_in_current_csi_version (){ |
There was a problem hiding this comment.
exist*
I would also use for instead of in
| csv_files=$(get_csv_files) | ||
| for csv_file in $csv_files | ||
| do | ||
| yq eval-all 'select(fileIndex==0).spec.install.spec.clusterPermissions[0].rules = select(fileIndex==1).rules | select(fi==0)' $csv_file config/rbac/role.yaml -i |
| are_csv_files_exsists_in_current_csi_version (){ | ||
| current_csi_version=$(get_current_csi_version) | ||
| if ! compgen -G "${PWD}/deploy/olm-catalog/*/$current_csi_version" > /dev/null; then | ||
| exit 0 |
There was a problem hiding this comment.
the function name implies it is a boolean function, but this line shows that the function is not boolean.
please change this function to be boolean
| } | ||
|
|
||
| align_roles (){ | ||
| are_csv_files_exsists_in_current_csi_version |
There was a problem hiding this comment.
this function call seems to be duplicated. please move it outside
This PR is also related to CSI-3315