From b0aab1394a434b02162886da9c06bf2611ceb3ef Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Thu, 27 Jul 2023 11:00:20 +0900 Subject: [PATCH 1/6] Add pre-commit config file ... so that we can run the validation in CI later. This also fixes a few errors detected atm. --- .golangci.yaml | 18 +++++++++++ .pre-commit-config.yaml | 62 ++++++++++++++++++++++++++++++++++++++ README.md | 1 - hack/boilerplate.go.txt | 2 +- templates/common/common.sh | 2 +- 5 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 .golangci.yaml create mode 100644 .pre-commit-config.yaml diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 0000000..5ed88ce --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,18 @@ +linters: + # Enable specific linter + # https://golangci-lint.run/usage/linters/#enabled-by-default + enable: + - errorlint + - revive + - ginkgolinter + - gofmt + - govet +linters-settings: + revive: + rules: + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unused-parameter + - name: unused-parameter + severity: warning + disabled: true +run: + timeout: 5m diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..4312239 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,62 @@ +repos: +- repo: local + hooks: + - id: gotidy + name: gotidy + language: system + entry: make + args: ["tidy"] + pass_filenames: false + - id: make-manifests + name: make-manifests + language: system + entry: make + args: ['manifests'] + pass_filenames: false + - id: make-generate + name: make-generate + language: system + entry: make + args: ['generate'] + pass_filenames: false + - id: make-operator-lint + name: make-operator-lint + language: system + entry: make + args: ['operator-lint'] + pass_filenames: false + +- repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.4.0 + hooks: + - id: check-added-large-files + - id: fix-byte-order-marker + - id: check-case-conflict + - id: check-executables-have-shebangs + exclude: ^vendor + - id: check-shebang-scripts-are-executable + exclude: ^vendor + - id: check-merge-conflict + - id: check-symlinks + - id: destroyed-symlinks + - id: check-yaml + args: [-m] + - id: check-json + - id: detect-private-key + - id: end-of-file-fixer + exclude: ^vendor + - id: no-commit-to-branch + - id: trailing-whitespace + exclude: ^vendor + +- repo: https://github.com/openstack/bashate.git + rev: 2.1.1 + hooks: + - id: bashate + entry: bashate --error . --ignore=E006,E040,E011,E020,E012 + +- repo: https://github.com/golangci/golangci-lint + rev: v1.52.2 + hooks: + - id: golangci-lint + args: ["-v"] diff --git a/README.md b/README.md index 6b92982..54c82e0 100644 --- a/README.md +++ b/README.md @@ -91,4 +91,3 @@ distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. - diff --git a/hack/boilerplate.go.txt b/hack/boilerplate.go.txt index 65b8622..6975adb 100644 --- a/hack/boilerplate.go.txt +++ b/hack/boilerplate.go.txt @@ -12,4 +12,4 @@ distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. -*/ \ No newline at end of file +*/ diff --git a/templates/common/common.sh b/templates/common/common.sh index 4bda950..7b37e74 100755 --- a/templates/common/common.sh +++ b/templates/common/common.sh @@ -32,4 +32,4 @@ function merge_config_dir { cp -f ${conf} /var/lib/config-data/merged/ fi done -} \ No newline at end of file +} From 893a591e4927592d6f0869911ebdd70483b6eb04 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Fri, 9 Jun 2023 12:26:38 +0900 Subject: [PATCH 2/6] Remove endpoint url and keystone service id from status These items presented in CR status are not really used. Endpoints can be discovered using Keystone and we have no use case to require service id. --- api/v1beta1/barbican_types.go | 8 ------- api/v1beta1/zz_generated.deepcopy.go | 24 ------------------- .../barbican.openstack.org_barbicans.yaml | 14 ----------- controllers/barbican_controller.go | 3 --- 4 files changed, 49 deletions(-) diff --git a/api/v1beta1/barbican_types.go b/api/v1beta1/barbican_types.go index 31bc608..7fabf9c 100644 --- a/api/v1beta1/barbican_types.go +++ b/api/v1beta1/barbican_types.go @@ -68,14 +68,6 @@ type BarbicanStatus struct { // Conditions Conditions condition.Conditions `json:"conditions,omitempty" optional:"true"` - // API endpoints - // TODO(dmendiza): This thing is hideous. Why do we need it? - APIEndpoints map[string]map[string]string `json:"apiEndpoints,omitempty"` - - // ServiceIDs - // TODO(dmendiza): This thing is hideous. Why do we need it? - ServiceIDs map[string]string `json:"serviceIDs,omitempty"` - // ReadyCount of Barbican API instances BarbicanAPIReadyCount int32 `json:"barbicanAPIReadyCount,omitempty"` diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 98a6254..4f07c30 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -297,30 +297,6 @@ func (in *BarbicanStatus) DeepCopyInto(out *BarbicanStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.APIEndpoints != nil { - in, out := &in.APIEndpoints, &out.APIEndpoints - *out = make(map[string]map[string]string, len(*in)) - for key, val := range *in { - var outVal map[string]string - if val == nil { - (*out)[key] = nil - } else { - in, out := &val, &outVal - *out = make(map[string]string, len(*in)) - for key, val := range *in { - (*out)[key] = val - } - } - (*out)[key] = outVal - } - } - if in.ServiceIDs != nil { - in, out := &in.ServiceIDs, &out.ServiceIDs - *out = make(map[string]string, len(*in)) - for key, val := range *in { - (*out)[key] = val - } - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BarbicanStatus. diff --git a/config/crd/bases/barbican.openstack.org_barbicans.yaml b/config/crd/bases/barbican.openstack.org_barbicans.yaml index 65a963d..beae135 100644 --- a/config/crd/bases/barbican.openstack.org_barbicans.yaml +++ b/config/crd/bases/barbican.openstack.org_barbicans.yaml @@ -282,14 +282,6 @@ spec: status: description: BarbicanStatus defines the observed state of Barbican properties: - apiEndpoints: - additionalProperties: - additionalProperties: - type: string - type: object - description: 'API endpoints TODO(dmendiza): This thing is hideous. Why - do we need it?' - type: object barbicanAPIReadyCount: description: ReadyCount of Barbican API instances format: int32 @@ -349,12 +341,6 @@ spec: type: string description: Map of hashes to track e.g. job status type: object - serviceIDs: - additionalProperties: - type: string - description: 'ServiceIDs TODO(dmendiza): This thing is hideous. Why - do we need it?' - type: object transportURLSecret: description: TransportURLSecret - Secret containing RabbitMQ transportURL type: string diff --git a/controllers/barbican_controller.go b/controllers/barbican_controller.go index 872aa11..0d47e3e 100644 --- a/controllers/barbican_controller.go +++ b/controllers/barbican_controller.go @@ -138,9 +138,6 @@ func (r *BarbicanReconciler) Reconcile(ctx context.Context, req ctrl.Request) (r if instance.Status.Hash == nil { instance.Status.Hash = map[string]string{} } - if instance.Status.APIEndpoints == nil { - instance.Status.APIEndpoints = map[string]map[string]string{} - } // Handle service delete if !instance.DeletionTimestamp.IsZero() { From 6b21ac4264d4ecf1432b205ecd1b5791e12d9bbd Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Thu, 13 Jul 2023 15:33:21 +0900 Subject: [PATCH 3/6] Fix defaulting of Replicas spec Currently the default values are ignored because these specs are set to 0 by webhook. Update the type from int32 to *int32 to avoid that behavior and ensure the expected default values are used. This also removes the large replicas set in the sample config, because these are not suitable for testing. --- api/v1beta1/common_types.go | 2 +- api/v1beta1/zz_generated.deepcopy.go | 5 +++++ config/samples/barbican_v1beta1_barbican.yaml | 2 -- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/api/v1beta1/common_types.go b/api/v1beta1/common_types.go index 74ce717..058e0f0 100644 --- a/api/v1beta1/common_types.go +++ b/api/v1beta1/common_types.go @@ -67,7 +67,7 @@ type BarbicanComponentTemplate struct { // +kubebuilder:validation:Maximum=32 // +kubebuilder:validation:Minimum=0 // Replicas of Barbican API to run - Replicas int32 `json:"replicas"` + Replicas *int32 `json:"replicas"` // +kubebuilder:validation:Optional // CustomServiceConfig - customize the service config using this parameter to change service defaults, diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 4f07c30..c4992bf 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -177,6 +177,11 @@ func (in *BarbicanComponentTemplate) DeepCopyInto(out *BarbicanComponentTemplate (*out)[key] = val } } + if in.Replicas != nil { + in, out := &in.Replicas, &out.Replicas + *out = new(int32) + **out = **in + } if in.DefaultConfigOverwrite != nil { in, out := &in.DefaultConfigOverwrite, &out.DefaultConfigOverwrite *out = make(map[string]string, len(*in)) diff --git a/config/samples/barbican_v1beta1_barbican.yaml b/config/samples/barbican_v1beta1_barbican.yaml index fdbce9a..bfae161 100644 --- a/config/samples/barbican_v1beta1_barbican.yaml +++ b/config/samples/barbican_v1beta1_barbican.yaml @@ -35,7 +35,6 @@ spec: containerImage: some_image nodeSelector: optional_override: here - replicas: 10 customServiceConfig: | [optional] overrides = True @@ -55,7 +54,6 @@ spec: containerImage: some_image nodeSelector: optional_override: here - replicas: 10 customServiceConfig: | [optional] overrides = True From 9e8cb65cccbc6de3e27ce59ceeb0af2ec6affc78 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Thu, 13 Jul 2023 15:34:51 +0900 Subject: [PATCH 4/6] Drop hard-coded image names from sample We should be able to deploy services using the sample file, so these should be point valid container images. Because the default images are set by webhook, we can remove these from samples now. --- config/samples/barbican_v1beta1_barbican.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/config/samples/barbican_v1beta1_barbican.yaml b/config/samples/barbican_v1beta1_barbican.yaml index bfae161..f012f5a 100644 --- a/config/samples/barbican_v1beta1_barbican.yaml +++ b/config/samples/barbican_v1beta1_barbican.yaml @@ -32,7 +32,6 @@ spec: policy.json: | {"some": "custom policy"} barbicanAPI: - containerImage: some_image nodeSelector: optional_override: here customServiceConfig: | @@ -51,7 +50,6 @@ spec: loadBalancerIPs: - some_ip_string barbicanWorker: - containerImage: some_image nodeSelector: optional_override: here customServiceConfig: | From ada76a46df5d2ac12f083a0b57382cbfacad7566 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Thu, 13 Jul 2023 15:36:49 +0900 Subject: [PATCH 5/6] Fix inconsistent password selector value for service password We use Password for service user passwords globally. --- api/v1beta1/common_types.go | 4 ++-- config/crd/bases/barbican.openstack.org_barbicanapis.yaml | 4 ++-- config/crd/bases/barbican.openstack.org_barbicans.yaml | 4 ++-- config/crd/bases/barbican.openstack.org_barbicanworkers.yaml | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/api/v1beta1/common_types.go b/api/v1beta1/common_types.go index 058e0f0..988444b 100644 --- a/api/v1beta1/common_types.go +++ b/api/v1beta1/common_types.go @@ -33,7 +33,7 @@ type BarbicanTemplate struct { Secret string `json:"secret"` // +kubebuilder:validation:Optional - // +kubebuilder:default={database: BarbicanDatabasePassword, service: BarbicanServiceUserPassword} + // +kubebuilder:default={database: BarbicanDatabasePassword, service: BarbicanPassword} // TODO(dmendiza): Maybe we'll add SimpleCrypto key here? // PasswordSelectors - Selectors to identify the DB and ServiceUser password from the Secret PasswordSelectors PasswordSelector `json:"passwordSelectors"` @@ -93,7 +93,7 @@ type PasswordSelector struct { // Database - Selector to get the barbican database user password from the Secret Database string `json:"database"` // +kubebuilder:validation:Optional - // +kubebuilder:default="BarbicanServiceUserPassword" + // +kubebuilder:default="BarbicanPassword" // Service - Selector to get the barbican service user password from the Secret Service string `json:"service"` diff --git a/config/crd/bases/barbican.openstack.org_barbicanapis.yaml b/config/crd/bases/barbican.openstack.org_barbicanapis.yaml index e81d68a..f6acd72 100644 --- a/config/crd/bases/barbican.openstack.org_barbicanapis.yaml +++ b/config/crd/bases/barbican.openstack.org_barbicanapis.yaml @@ -147,7 +147,7 @@ spec: passwordSelectors: default: database: BarbicanDatabasePassword - service: BarbicanServiceUserPassword + service: BarbicanPassword description: 'TODO(dmendiza): Maybe we''ll add SimpleCrypto key here? PasswordSelectors - Selectors to identify the DB and ServiceUser password from the Secret' @@ -158,7 +158,7 @@ spec: user password from the Secret type: string service: - default: BarbicanServiceUserPassword + default: BarbicanPassword description: Service - Selector to get the barbican service user password from the Secret type: string diff --git a/config/crd/bases/barbican.openstack.org_barbicans.yaml b/config/crd/bases/barbican.openstack.org_barbicans.yaml index beae135..5db87dd 100644 --- a/config/crd/bases/barbican.openstack.org_barbicans.yaml +++ b/config/crd/bases/barbican.openstack.org_barbicans.yaml @@ -230,7 +230,7 @@ spec: passwordSelectors: default: database: BarbicanDatabasePassword - service: BarbicanServiceUserPassword + service: BarbicanPassword description: 'TODO(dmendiza): Maybe we''ll add SimpleCrypto key here? PasswordSelectors - Selectors to identify the DB and ServiceUser password from the Secret' @@ -241,7 +241,7 @@ spec: user password from the Secret type: string service: - default: BarbicanServiceUserPassword + default: BarbicanPassword description: Service - Selector to get the barbican service user password from the Secret type: string diff --git a/config/crd/bases/barbican.openstack.org_barbicanworkers.yaml b/config/crd/bases/barbican.openstack.org_barbicanworkers.yaml index 97e9a07..db55ad2 100644 --- a/config/crd/bases/barbican.openstack.org_barbicanworkers.yaml +++ b/config/crd/bases/barbican.openstack.org_barbicanworkers.yaml @@ -104,7 +104,7 @@ spec: passwordSelectors: default: database: BarbicanDatabasePassword - service: BarbicanServiceUserPassword + service: BarbicanPassword description: 'TODO(dmendiza): Maybe we''ll add SimpleCrypto key here? PasswordSelectors - Selectors to identify the DB and ServiceUser password from the Secret' @@ -115,7 +115,7 @@ spec: user password from the Secret type: string service: - default: BarbicanServiceUserPassword + default: BarbicanPassword description: Service - Selector to get the barbican service user password from the Secret type: string From 23b0317c9b478e2ee86b0ad7b0d52e9300d3974a Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Thu, 6 Jul 2023 15:06:45 +0900 Subject: [PATCH 6/6] Watch RBAC resources to trigger reconcile This ensures the controller watches service account, role, and role bindings to reconcile these resources in case any change is made. --- controllers/barbican_controller.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/controllers/barbican_controller.go b/controllers/barbican_controller.go index 0d47e3e..ff3fdb2 100644 --- a/controllers/barbican_controller.go +++ b/controllers/barbican_controller.go @@ -44,6 +44,7 @@ import ( "github.com/openstack-k8s-operators/lib-common/modules/common/secret" "github.com/openstack-k8s-operators/lib-common/modules/common/util" "github.com/openstack-k8s-operators/lib-common/modules/database" + corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" k8s_errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -279,6 +280,9 @@ func (r *BarbicanReconciler) reconcileDelete(ctx context.Context, instance *barb func (r *BarbicanReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&barbicanv1beta1.Barbican{}). + Owns(&corev1.ServiceAccount{}). + Owns(&rbacv1.Role{}). + Owns(&rbacv1.RoleBinding{}). Complete(r) }