From 9dc3205476103a31d80aa934fbebf9557c8052e1 Mon Sep 17 00:00:00 2001 From: Alexandre Havrileck Date: Thu, 25 Jan 2024 14:03:37 +0100 Subject: [PATCH] fix: Upgrade linter and patch all code --- .github/workflows/ci.yml | 2 +- .golangci.yaml | 514 ++++++++++++++++++ .golangci.yml | 61 --- controllers/postgresql/postgres/aws.go | 4 + controllers/postgresql/postgres/azure.go | 1 + controllers/postgresql/postgres/database.go | 2 + .../postgresql/postgres/pool_manager.go | 16 +- controllers/postgresql/postgres/postgres.go | 7 +- controllers/postgresql/postgres/role.go | 3 +- .../postgresqldatabase_controller.go | 42 +- ...ostgresqlengineconfiguration_controller.go | 17 +- .../postgresql/postgresqluser_controller.go | 25 +- .../postgresqluserrole_controller.go | 51 +- controllers/utils/utils.go | 4 +- main.go | 14 +- 15 files changed, 633 insertions(+), 130 deletions(-) create mode 100644 .golangci.yaml delete mode 100644 .golangci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5fa7462..4d43bca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,7 +43,7 @@ jobs: - uses: golangci/golangci-lint-action@v3 with: # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version - version: v1.51.1 + version: v1.55.2 # Optional: working directory, useful for monorepos # working-directory: somedir diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 0000000..36e9e86 --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,514 @@ +# This file contains all available configuration options +# with their default values. +# options for analysis running +run: + go: "1.19" + # default concurrency is a available CPU number + concurrency: 4 + # timeout for analysis, e.g. 30s, 5m, default is 1m + timeout: 10m + # exit code when at least one issue was found, default is 1 + issues-exit-code: 1 + # include test files or not, default is true + tests: false + # list of build tags, all linters use it. Default is empty list. + build-tags: [] + # which dirs to skip: issues from them won't be reported; + # can use regexp here: generated.*, regexp is applied on full path; + # default value is empty list, but default dirs are skipped independently + # from this option's value (see skip-dirs-use-default). + skip-dirs: [] + # default is true. Enables skipping of directories: + # vendor$, third_party$, testdata$, examples$, Godeps$, builtin$ + skip-dirs-use-default: true + # which files to skip: they will be analyzed, but issues from them + # won't be reported. Default value is empty list, but there is + # no need to include all autogenerated files, we confidently recognize + # autogenerated files. If it's not please let us know. + skip-files: [] + # by default isn't set. If set we pass it to "go list -mod={option}". From "go help modules": + # If invoked with -mod=readonly, the go command is disallowed from the implicit + # automatic updating of go.mod described above. Instead, it fails when any changes + # to go.mod are needed. This setting is most useful to check that go.mod does + # not need updates, such as in a continuous integration and testing system. + # If invoked with -mod=vendor, the go command assumes that the vendor + # directory holds the correct copies of dependencies and ignores + # the dependency descriptions in go.mod. + # modules-download-mode: readonly|release|vendor + modules-download-mode: readonly + # Allow multiple parallel golangci-lint instances running. + # If false (default) - golangci-lint acquires file lock on start. + allow-parallel-runners: false +# output configuration options +output: + # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number" + format: colored-line-number + # print lines of code with issue, default is true + print-issued-lines: true + # print linter name in the end of issue text, default is true + print-linter-name: true + # make issues output unique by line, default is true + uniq-by-line: true + # add a prefix to the output file references; default is no prefix + path-prefix: "" +# all available settings of specific linters +linters-settings: + tagalign: + align: true + sort: true + godox: + # report any comments starting with keywords, this is useful for TODO or FIXME comments that + # might be left in the code accidentally and should be resolved before merging + keywords: # default keywords are TODO, BUG, and FIXME, these can be overwritten by this setting + - BUG + - FIXME + # - NOTE + # - OPTIMIZE # marks code that should be optimized before merging + # - HACK # marks hack-arounds that should be removed before merging + gofmt: + # simplify code: gofmt with `-s` option, true by default + simplify: true + govet: + # report about shadowed variables + check-shadowing: true + enable-all: true + revive: + # When set to false, ignores files with "GENERATED" header, similar to golint. + # See https://github.com/mgechev/revive#available-rules for details. + # Default: false + ignore-generated-header: true + rules: + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#atomic + - name: atomic + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#blank-imports + - name: blank-imports + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#bool-literal-in-expr + - name: bool-literal-in-expr + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#call-to-gc + - name: call-to-gc + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#confusing-results + - name: confusing-results + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#constant-logical-expr + - name: constant-logical-expr + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#context-as-argument + - name: context-as-argument + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#context-keys-type + - name: context-keys-type + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#datarace + - name: datarace + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#deep-exit + - name: deep-exit + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#defer + - name: defer + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#dot-imports + - name: dot-imports + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#duplicated-imports + - name: duplicated-imports + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#early-return + - name: early-return + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#empty-block + - name: empty-block + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#empty-lines + - name: empty-lines + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#error-naming + - name: error-naming + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#error-return + - name: error-return + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#error-strings + - name: error-strings + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#errorf + - name: errorf + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#function-result-limit + - name: function-result-limit + severity: warning + disabled: false + arguments: [4] + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#get-return + - name: get-return + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#identical-branches + - name: identical-branches + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#if-return + - name: if-return + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#increment-decrement + - name: increment-decrement + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#indent-error-flow + - name: indent-error-flow + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#import-shadowing + - name: import-shadowing + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#modifies-parameter + - name: modifies-parameter + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#modifies-value-receiver + - name: modifies-value-receiver + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#nested-structs + - name: nested-structs + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#optimize-operands-order + - name: optimize-operands-order + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#package-comments + - name: package-comments + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#range + - name: range + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#range-val-in-closure + - name: range-val-in-closure + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#range-val-address + - name: range-val-address + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#receiver-naming + - name: receiver-naming + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#redefines-builtin-id + - name: redefines-builtin-id + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#string-of-int + - name: string-of-int + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#superfluous-else + - name: superfluous-else + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#time-equal + - name: time-equal + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#time-naming + - name: time-naming + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#var-declaration + - name: var-declaration + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unconditional-recursion + - name: unconditional-recursion + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unexported-naming + - name: unexported-naming + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unexported-return + - name: unexported-return + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unnecessary-stmt + - name: unnecessary-stmt + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unreachable-code + - name: unreachable-code + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unused-parameter + - name: unused-parameter + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unused-receiver + - name: unused-receiver + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#useless-break + - name: useless-break + severity: warning + disabled: false + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#waitgroup-by-value + - name: waitgroup-by-value + severity: warning + disabled: false + gosec: + # To select a subset of rules to run. + # Available rules: https://github.com/securego/gosec#available-rules + # Default: [] - means include all rules + includes: + - G101 # Look for hard coded credentials + - G102 # Bind to all interfaces + - G103 # Audit the use of unsafe block + - G104 # Audit errors not checked + - G106 # Audit the use of ssh.InsecureIgnoreHostKey + - G107 # Url provided to HTTP request as taint input + - G108 # Profiling endpoint automatically exposed on /debug/pprof + - G109 # Potential Integer overflow made by strconv.Atoi result conversion to int16/32 + - G110 # Potential DoS vulnerability via decompression bomb + - G111 # Potential directory traversal + - G112 # Potential slowloris attack + - G113 # Usage of Rat.SetString in math/big with an overflow (CVE-2022-23772) + - G114 # Use of net/http serve function that has no support for setting timeouts + - G201 # SQL query construction using format string + - G202 # SQL query construction using string concatenation + - G203 # Use of unescaped data in HTML templates + - G204 # Audit use of command execution + - G301 # Poor file permissions used when creating a directory + - G302 # Poor file permissions used with chmod + - G303 # Creating tempfile using a predictable path + - G304 # File path provided as taint input + - G305 # File traversal when extracting zip/tar archive + - G306 # Poor file permissions used when writing to a new file + - G307 # Deferring a method which returns an error + - G401 # Detect the usage of DES, RC4, MD5 or SHA1 + - G402 # Look for bad TLS connection settings + - G403 # Ensure minimum RSA key length of 2048 bits + - G404 # Insecure random number source (rand) + - G501 # Import blocklist: crypto/md5 + - G502 # Import blocklist: crypto/des + - G503 # Import blocklist: crypto/rc4 + - G504 # Import blocklist: net/http/cgi + - G505 # Import blocklist: crypto/sha1 + - G601 # Implicit memory aliasing of items from a range statement + lll: + # max line length, lines longer will be reported. Default is 120. + # '\t' is counted as 1 character by default, and can be changed with the tab-width option + line-length: 250 + # tab width in spaces. Default to 1. + tab-width: 1 + testpackage: + # regexp pattern to skip files + skip-regexp: _test\.go +linters: + enable: + - govet + - errcheck + - staticcheck + - unused + - gosimple + - ineffassign + - typecheck + - bodyclose + - noctx + - rowserrcheck + - stylecheck + - gosec + - unconvert + - goconst + - asciicheck + - gofmt + - goimports + - goheader + - misspell + - lll + - unparam + - dogsled + - nakedret + - gocritic + - gochecknoinits + - godox + - whitespace + - wsl + - goprintffuncname + - gomnd + - gomodguard + - godot + - exportloopref + - sqlclosecheck + - nlreturn + - nolintlint + - forcetypeassert + - gomoddirectives + - importas + - nilerr + - promlinter + - revive + - wastedassign + - errname + - tagliatelle + - bidichk + - contextcheck + - tenv + - containedctx + - errchkjson + - decorder + - grouper + - asasalint + - execinquery + - reassign + - usestdlibvars + - tagalign + - gosmopolitan + - mirror + - gochecksumtype + - inamedparam + - perfsprint + - protogetter + - sloglint + - testifylint + disable: + - exhaustive + - nilnil + - deadcode + - dupl + - varcheck + - structcheck + - prealloc + - gochecknoglobals + - gocyclo + - gocognit + - gofumpt + - funlen + - testpackage + - gci + - goerr113 # Forbid dynamic errors + - nestif + - scopelint + - ireturn + - varnamelen + - maintidx + - exhaustruct + - nosnakecase + - interfacebloat + - zerologlint + - depguard + fast: false +issues: + exclude-rules: + # Exclude lll issues for long lines with go:generate + - linters: + - lll + source: "^//go:generate " + - linters: + - lll + - gocritic + - govet + - dupl + - gochecknoinits + path: \_types\.go + - linters: + - gochecknoinits + path: main.go + # issues: + # # List of regexps of issue texts to exclude, empty list by default. + # # But independently from this option we use default exclude patterns, + # # it can be disabled by `exclude-use-default: false`. To list all + # # excluded by default patterns execute `golangci-lint run --help` + # exclude: + # - abcdef + # # Excluding configuration per-path, per-linter, per-text and per-source + # exclude-rules: + # # Exclude some linters from running on tests files. + # - path: _test\.go + # linters: + # - gocyclo + # - errcheck + # - dupl + # - gosec + # # Exclude known linters from partially hard-vendored code, + # # which is impossible to exclude via "nolint" comments. + # - path: internal/hmac/ + # text: "weak cryptographic primitive" + # linters: + # - gosec + # # Exclude some staticcheck messages + # - linters: + # - staticcheck + # text: "SA9003:" + # # Exclude lll issues for long lines with go:generate + # - linters: + # - lll + # source: "^//go:generate " + # # Independently from option `exclude` we use default exclude patterns, + # # it can be disabled by this option. To list all + # # excluded by default patterns execute `golangci-lint run --help`. + # # Default value for this option is true. + # exclude-use-default: false + # # Maximum issues count per one linter. Set to 0 to disable. Default is 50. + # max-issues-per-linter: 0 + # # Maximum count of issues with the same text. Set to 0 to disable. Default is 3. + # max-same-issues: 0 + # # Show only new issues: if there are unstaged changes or untracked files, + # # only those changes are analyzed, else only changes in HEAD~ are analyzed. + # # It's a super-useful option for integration of golangci-lint into existing + # # large codebase. It's not practical to fix all existing issues at the moment + # # of integration: much better don't allow issues in new code. + # # Default is false. + # new: false + # # Show only new issues created in git patch with set file path. + # new-from-patch: # path/to/patch/file + # Fix found issues (if it's supported by the linter) + fix: true +severity: + # Default value is empty string. + # Set the default severity for issues. If severity rules are defined and the issues + # do not match or no severity is provided to the rule this will be the default + # severity applied. Severities should match the supported severity names of the + # selected out format. + # - Code climate: https://docs.codeclimate.com/docs/issues#issue-severity + # - Checkstyle: https://checkstyle.sourceforge.io/property_types.html#severity + # - Github: https://help.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-error-message + default-severity: error + # The default value is false. + # If set to true severity-rules regular expressions become case sensitive. + case-sensitive: false + # Default value is empty list. + # When a list of severity rules are provided, severity information will be added to lint + # issues. Severity rules have the same filtering capability as exclude rules except you + # are allowed to specify one matcher per severity rule. + # Only affects out formats that support setting severity information. + rules: + - linters: + - dupl + severity: info diff --git a/.golangci.yml b/.golangci.yml deleted file mode 100644 index 5ee3487..0000000 --- a/.golangci.yml +++ /dev/null @@ -1,61 +0,0 @@ -linters-settings: - golint: - # set the confidence level of a problem before it is reported. Setting it to 0 will show every issue from golint. - min-confidence: 0 - gocyclo: - # a minimal complexity of function to report it - min-complexity: 70 - gocognit: - # minimal code complexity to report - min-complexity: 70 - goconst: - # only report strings with the minimum given length - min-len: 2 - # report from how many occurrence - min-occurrences: 2 - funlen: - # We have very long type definitions for K8s resources - lines: 250 - statements: 100 - -run: - # Skip lint on tests files - tests: false - timeout: 10m - -linters: - enable-all: true - disable: - - dupl - - maligned - - unparam - - lll - - gochecknoinits - - gochecknoglobals - - wsl - - godox - - gocritic - - wrapcheck - - varnamelen - - nestif - - nosprintfhostport - - nilnil - - goerr113 - - ireturn - - gci - - ifshort - - forcetypeassert - - exhaustivestruct - - exhaustruct - - exhaustive - - errorlint - - cyclop - - maintidx - - gofumpt - - scopelint - - interfacer - - golint - - rowserrcheck - - sqlclosecheck - - structcheck - - wastedassign diff --git a/controllers/postgresql/postgres/aws.go b/controllers/postgresql/postgres/aws.go index 8ab84b0..f7a3320 100644 --- a/controllers/postgresql/postgres/aws.go +++ b/controllers/postgresql/postgres/aws.go @@ -75,9 +75,11 @@ func (c *awspg) DropRoleAndDropAndChangeOwnedBy(role, newOwner, database string) if !ok { return err } + if pqErr.Code == RoleNotFoundErrorCode { return nil } + if pqErr.Code != InvalidGrantOperationErrorCode { return err } @@ -125,9 +127,11 @@ func (c *awspg) ChangeAndDropOwnedBy(role, newOwner, database string) error { if !ok { return err } + if pqErr.Code == RoleNotFoundErrorCode { return nil } + if pqErr.Code != InvalidGrantOperationErrorCode { return err } diff --git a/controllers/postgresql/postgres/azure.go b/controllers/postgresql/postgres/azure.go index 8f5763d..0d5fc0d 100644 --- a/controllers/postgresql/postgres/azure.go +++ b/controllers/postgresql/postgres/azure.go @@ -15,6 +15,7 @@ const MinUserSplit = 1 func newAzurePG(postgres *pg) PG { splitUser := strings.Split(postgres.user, "@") serverName := "" + if len(splitUser) > MinUserSplit { serverName = splitUser[1] } diff --git a/controllers/postgresql/postgres/database.go b/controllers/postgresql/postgres/database.go index 1d5aa75..62c955c 100644 --- a/controllers/postgresql/postgres/database.go +++ b/controllers/postgresql/postgres/database.go @@ -120,6 +120,7 @@ func (c *pg) DropExtension(database, extension string, cascade bool) error { if cascade { param = CascadeKeyword } + _, err = c.db.Exec(fmt.Sprintf(DropExtensionSQLTemplate, extension, param)) if err != nil { return err @@ -140,6 +141,7 @@ func (c *pg) DropSchema(database, schema string, cascade bool) error { if cascade { param = CascadeKeyword } + _, err = c.db.Exec(fmt.Sprintf(DropSchemaSQLTemplate, schema, param)) if err != nil { return err diff --git a/controllers/postgresql/postgres/pool_manager.go b/controllers/postgresql/postgres/pool_manager.go index 5eeb1e9..48868b1 100644 --- a/controllers/postgresql/postgres/pool_manager.go +++ b/controllers/postgresql/postgres/pool_manager.go @@ -14,11 +14,11 @@ const ( // Pool saved structure per postgres engine configuration. type poolSaved struct { + // This map will save all pools per database + pools *sync.Map // Username and password are saved because this comes from secret username string password string - // This map will save all pools per database - pools *sync.Map } // Pool manager map per pgec. @@ -53,7 +53,7 @@ func getOrOpenPool(p *pg, database string) (*sql.DB, error) { db = sqlDB } else { // Cast saved pool object - sav := savInt.(*poolSaved) + sav, _ := savInt.(*poolSaved) // Check if username and password haven't changed, if yes, close pools and recreate current if sav.username != p.GetUser() || sav.password != p.GetPassword() { // Close all pools @@ -81,7 +81,7 @@ func getOrOpenPool(p *pg, database string) (*sql.DB, error) { db = sqlDB } else { // Result - db = sqlDBInt.(*sql.DB) + db, _ = sqlDBInt.(*sql.DB) } } @@ -125,7 +125,7 @@ func CloseDatabaseSavedPoolsForName(name, database string) error { } // Cast pool saved - ps := psInt.(*poolSaved) + ps, _ := psInt.(*poolSaved) // Get entry enInt, ok := ps.pools.Load(database) @@ -135,7 +135,7 @@ func CloseDatabaseSavedPoolsForName(name, database string) error { } // Cast db - en := enInt.(*sql.DB) + en, _ := enInt.(*sql.DB) // Close pool err := en.Close() @@ -159,7 +159,7 @@ func CloseAllSavedPoolsForName(name string) error { } // Cast pool saved - ps := psInt.(*poolSaved) + ps, _ := psInt.(*poolSaved) // Save all keys to be removed keysToBeRemoved := make([]interface{}, 0) @@ -168,7 +168,7 @@ func CloseAllSavedPoolsForName(name string) error { // Loop over pools ps.pools.Range(func(k, val interface{}) bool { // Cast sql db - v := val.(*sql.DB) + v, _ := val.(*sql.DB) // Close pool err = v.Close() // Check error diff --git a/controllers/postgresql/postgres/postgres.go b/controllers/postgresql/postgres/postgres.go index 38c19b7..6189054 100644 --- a/controllers/postgresql/postgres/postgres.go +++ b/controllers/postgresql/postgres/postgres.go @@ -53,12 +53,12 @@ type pg struct { db *sql.DB log logr.Logger host string - port int user string pass string args string defaultDatabase string name string + port int } func NewPG( @@ -139,6 +139,7 @@ func (c *pg) Ping() error { if err != nil { return err } + err = c.db.Ping() if err != nil { return err @@ -147,8 +148,8 @@ func (c *pg) Ping() error { return nil } -func TemplatePostgresqlURLWithArgs(host, user, password, URIArgs, database string, port int) string { - return fmt.Sprintf("postgresql://%s:%s@%s:%d/%s?%s", user, password, host, port, database, URIArgs) +func TemplatePostgresqlURLWithArgs(host, user, password, uriArgs, database string, port int) string { + return fmt.Sprintf("postgresql://%s:%s@%s:%d/%s?%s", user, password, host, port, database, uriArgs) } func TemplatePostgresqlURL(host, user, password, database string, port int) string { diff --git a/controllers/postgresql/postgres/role.go b/controllers/postgresql/postgres/role.go index 4fd58c5..09d59e4 100644 --- a/controllers/postgresql/postgres/role.go +++ b/controllers/postgresql/postgres/role.go @@ -24,7 +24,7 @@ const ( GetRoleMembershipSQLTemplate = `SELECT r1.rolname as "role" FROM pg_catalog.pg_roles r JOIN pg_catalog.pg_auth_members m ON (m.member = r.oid) JOIN pg_roles r1 ON (m.roleid=r1.oid) WHERE r.rolcanlogin AND r.rolname='%s'` // DO NOT TOUCH THIS // Cannot filter on compute value so... cf line before. - GetRoleSettingsSQLTemplate = `SELECT pg_catalog.split_part(pg_catalog.unnest(setconfig), '=', 1) as parameter_type, pg_catalog.split_part(pg_catalog.unnest(setconfig), '=', 2) as parameter_value, d.datname as database FROM pg_catalog.pg_roles r JOIN pg_catalog.pg_db_role_setting c ON (c.setrole = r.oid) JOIN pg_catalog.pg_database d ON (d.oid = c.setdatabase) WHERE r.rolcanlogin AND r.rolname='%s'` + GetRoleSettingsSQLTemplate = `SELECT pg_catalog.split_part(pg_catalog.unnest(setconfig), '=', 1) as parameter_type, pg_catalog.split_part(pg_catalog.unnest(setconfig), '=', 2) as parameter_value, d.datname as database FROM pg_catalog.pg_roles r JOIN pg_catalog.pg_db_role_setting c ON (c.setrole = r.oid) JOIN pg_catalog.pg_database d ON (d.oid = c.setdatabase) WHERE r.rolcanlogin AND r.rolname='%s'` //nolint:lll//Because DoesRoleHaveActiveSessionSQLTemplate = `SELECT 1 from pg_stat_activity WHERE usename = '%s' group by usename` DuplicateRoleErrorCode = "42710" RoleNotFoundErrorCode = "42704" @@ -266,6 +266,7 @@ func (c *pg) DropRole(role string) error { if err != nil { return err } + _, err = c.db.Exec(fmt.Sprintf(DropRoleSQLTemplate, role)) // Check if error exists and if different from "ROLE NOT FOUND" => 42704 if err != nil { diff --git a/controllers/postgresql/postgresqldatabase_controller.go b/controllers/postgresql/postgresqldatabase_controller.go index 9198e5e..9b6a0f8 100644 --- a/controllers/postgresql/postgresqldatabase_controller.go +++ b/controllers/postgresql/postgresqldatabase_controller.go @@ -44,7 +44,7 @@ const ( ) // PostgresqlDatabaseReconciler reconciles a PostgresqlDatabase object. -type PostgresqlDatabaseReconciler struct { //nolint: golint,revive // generated +type PostgresqlDatabaseReconciler struct { //nolint: golint // generated client.Client Scheme *runtime.Scheme Recorder record.EventRecorder @@ -65,7 +65,7 @@ type PostgresqlDatabaseReconciler struct { //nolint: golint,revive // generated // // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.12.1/pkg/reconcile -func (r *PostgresqlDatabaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { +func (r *PostgresqlDatabaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint:wsl // it is like that // Issue with this logger: controller and controllerKind are incorrect // Build another logger from upper to fix this. // reqLogger := log.FromContext(ctx) @@ -75,6 +75,7 @@ func (r *PostgresqlDatabaseReconciler) Reconcile(ctx context.Context, req ctrl.R // Fetch the PostgresqlDatabase instance instance := &postgresqlv1alpha1.PostgresqlDatabase{} + err := r.Get(ctx, req.NamespacedName, instance) if err != nil { if errors.IsNotFound(err) { @@ -94,14 +95,14 @@ func (r *PostgresqlDatabaseReconciler) Reconcile(ctx context.Context, req ctrl.R if !instance.GetDeletionTimestamp().IsZero() { // Deletion in progress detected // Test should delete database - shouldDelete, err := r.shouldDropDatabase(ctx, instance) + shouldDelete, err := r.shouldDropDatabase(ctx, instance) //nolint:govet // Shadow err if err != nil { return r.manageError(ctx, reqLogger, instance, originalPatch, err) } // Check if should delete database is flagged if shouldDelete { // Drop database - err := r.manageDropDatabase(ctx, reqLogger, instance) + err = r.manageDropDatabase(ctx, reqLogger, instance) if err != nil { return r.manageError(ctx, reqLogger, instance, originalPatch, err) } @@ -150,7 +151,7 @@ func (r *PostgresqlDatabaseReconciler) Reconcile(ctx context.Context, req ctrl.R } // Add finalizer and owners - updated, err := r.updateInstance(ctx, instance, pgEngCfg) + updated, err := r.updateInstance(ctx, instance) // Check error if err != nil { return r.manageError(ctx, reqLogger, instance, originalPatch, err) @@ -171,6 +172,7 @@ func (r *PostgresqlDatabaseReconciler) Reconcile(ctx context.Context, req ctrl.R if owner == "" { owner = fmt.Sprintf("%s-owner", instance.Spec.Database) } + reader := fmt.Sprintf("%s-reader", instance.Spec.Database) writer := fmt.Sprintf("%s-writer", instance.Spec.Database) @@ -180,11 +182,13 @@ func (r *PostgresqlDatabaseReconciler) Reconcile(ctx context.Context, req ctrl.R return r.manageError(ctx, reqLogger, instance, originalPatch, errors.NewBadRequest(errStr)) } + if len(reader) > postgres.MaxIdentifierLength { errStr := fmt.Sprintf("identifier too long, must be <= 63, %s is %d character, must reduce database name length", reader, len(reader)) return r.manageError(ctx, reqLogger, instance, originalPatch, errors.NewBadRequest(errStr)) } + if len(writer) > postgres.MaxIdentifierLength { errStr := fmt.Sprintf("identifier too long, must be <= 63, %s is %d character, must reduce database name length", writer, len(writer)) @@ -230,7 +234,7 @@ func (r *PostgresqlDatabaseReconciler) Reconcile(ctx context.Context, req ctrl.R return r.manageSuccess(ctx, reqLogger, instance, originalPatch) } -func (r *PostgresqlDatabaseReconciler) manageDBCreationOrUpdate(pg postgres.PG, instance *postgresqlv1alpha1.PostgresqlDatabase, owner string) error { +func (*PostgresqlDatabaseReconciler) manageDBCreationOrUpdate(pg postgres.PG, instance *postgresqlv1alpha1.PostgresqlDatabase, owner string) error { // Check if database was already created in the past if instance.Status.Database != "" { // Check if database already exists @@ -354,20 +358,23 @@ func (r *PostgresqlDatabaseReconciler) shouldDropDatabase( if err != nil { return false, err } + if existingUser != nil { // Wait for children removal - err := fmt.Errorf("cannot remove resource because found user %s in namespace %s linked to this resource and wait for deletion flag is enabled", existingUser.Name, existingUser.Namespace) + err = fmt.Errorf("cannot remove resource because found user %s in namespace %s linked to this resource and wait for deletion flag is enabled", existingUser.Name, existingUser.Namespace) return false, err } + // Check if there are user role linked resource linked to this existingUserRole, err := r.getAnyUserRoleLinked(ctx, instance) if err != nil { return false, err } + if existingUserRole != nil { // Wait for children removal - err := fmt.Errorf("cannot remove resource because found user role %s in namespace %s linked to this resource and wait for deletion flag is enabled", existingUserRole.Name, existingUserRole.Namespace) + err = fmt.Errorf("cannot remove resource because found user role %s in namespace %s linked to this resource and wait for deletion flag is enabled", existingUserRole.Name, existingUserRole.Namespace) return false, err } @@ -431,7 +438,6 @@ func (r *PostgresqlDatabaseReconciler) getAnyUserLinked( func (r *PostgresqlDatabaseReconciler) updateInstance( ctx context.Context, instance *postgresqlv1alpha1.PostgresqlDatabase, - pgEngCfg *postgresqlv1alpha1.PostgresqlEngineConfiguration, ) (bool, error) { // Deep copy oCopy := instance.DeepCopy() @@ -447,7 +453,7 @@ func (r *PostgresqlDatabaseReconciler) updateInstance( return false, nil } -func (r *PostgresqlDatabaseReconciler) manageSchemas(pg postgres.PG, instance *postgresqlv1alpha1.PostgresqlDatabase) error { +func (*PostgresqlDatabaseReconciler) manageSchemas(pg postgres.PG, instance *postgresqlv1alpha1.PostgresqlDatabase) error { // Check if were deleted from list and asked to be deleted if instance.Status.Schemas != nil && instance.Spec.Schemas.DropOnOnDelete { newStatusSchemas := make([]string, 0) @@ -477,6 +483,7 @@ func (r *PostgresqlDatabaseReconciler) manageSchemas(pg postgres.PG, instance *p reader = instance.Status.Roles.Reader writer = instance.Status.Roles.Writer ) + for _, schema := range instance.Spec.Schemas.List { // Create schema err := pg.CreateSchema(instance.Spec.Database, owner, schema) @@ -489,6 +496,7 @@ func (r *PostgresqlDatabaseReconciler) manageSchemas(pg postgres.PG, instance *p if err != nil { return err } + err = pg.SetSchemaPrivileges(instance.Spec.Database, owner, writer, schema, writerPrivs) if err != nil { return err @@ -503,7 +511,7 @@ func (r *PostgresqlDatabaseReconciler) manageSchemas(pg postgres.PG, instance *p return nil } -func (r *PostgresqlDatabaseReconciler) manageExtensions(pg postgres.PG, instance *postgresqlv1alpha1.PostgresqlDatabase) error { +func (*PostgresqlDatabaseReconciler) manageExtensions(pg postgres.PG, instance *postgresqlv1alpha1.PostgresqlDatabase) error { // Check if were deleted from list and asked to be deleted if instance.Status.Extensions != nil && instance.Spec.Extensions.DropOnOnDelete { newStatusExtensions := make([]string, 0) @@ -543,7 +551,7 @@ func (r *PostgresqlDatabaseReconciler) manageExtensions(pg postgres.PG, instance return nil } -func (r *PostgresqlDatabaseReconciler) manageReaderRole(pg postgres.PG, reader string, instance *postgresqlv1alpha1.PostgresqlDatabase) error { +func (*PostgresqlDatabaseReconciler) manageReaderRole(pg postgres.PG, reader string, instance *postgresqlv1alpha1.PostgresqlDatabase) error { // Check if role was already created in the past if instance.Status.Roles.Reader != "" { // Check if role doesn't already exists @@ -572,7 +580,7 @@ func (r *PostgresqlDatabaseReconciler) manageReaderRole(pg postgres.PG, reader s // Check if exists if !exists { // Create it - err := pg.CreateGroupRole(reader) + err = pg.CreateGroupRole(reader) // Check error if err != nil { return err @@ -592,7 +600,7 @@ func (r *PostgresqlDatabaseReconciler) manageReaderRole(pg postgres.PG, reader s return nil } -func (r *PostgresqlDatabaseReconciler) manageWriterRole(pg postgres.PG, writer string, instance *postgresqlv1alpha1.PostgresqlDatabase) error { +func (*PostgresqlDatabaseReconciler) manageWriterRole(pg postgres.PG, writer string, instance *postgresqlv1alpha1.PostgresqlDatabase) error { // Check if role was already created in the past if instance.Status.Roles.Writer != "" { // Check if role doesn't already exists @@ -621,7 +629,7 @@ func (r *PostgresqlDatabaseReconciler) manageWriterRole(pg postgres.PG, writer s // Check if exists if !exists { // Create it - err := pg.CreateGroupRole(writer) + err = pg.CreateGroupRole(writer) // Check error if err != nil { return err @@ -641,7 +649,7 @@ func (r *PostgresqlDatabaseReconciler) manageWriterRole(pg postgres.PG, writer s return nil } -func (r *PostgresqlDatabaseReconciler) manageOwnerRole(pg postgres.PG, owner string, instance *postgresqlv1alpha1.PostgresqlDatabase) error { +func (*PostgresqlDatabaseReconciler) manageOwnerRole(pg postgres.PG, owner string, instance *postgresqlv1alpha1.PostgresqlDatabase) error { // Check if role was already created in the past if instance.Status.Roles.Owner != "" { // Check if role doesn't already exists @@ -670,7 +678,7 @@ func (r *PostgresqlDatabaseReconciler) manageOwnerRole(pg postgres.PG, owner str // Check if exists if !exists { // Create it - err := pg.CreateGroupRole(owner) + err = pg.CreateGroupRole(owner) // Check error if err != nil { return err diff --git a/controllers/postgresql/postgresqlengineconfiguration_controller.go b/controllers/postgresql/postgresqlengineconfiguration_controller.go index 6a893fe..f170c4a 100644 --- a/controllers/postgresql/postgresqlengineconfiguration_controller.go +++ b/controllers/postgresql/postgresqlengineconfiguration_controller.go @@ -41,7 +41,7 @@ const ( ) // PostgresqlEngineConfigurationReconciler reconciles a PostgresqlEngineConfiguration object. -type PostgresqlEngineConfigurationReconciler struct { //nolint: golint,revive // generated +type PostgresqlEngineConfigurationReconciler struct { //nolint: golint // generated client.Client Scheme *runtime.Scheme Recorder record.EventRecorder @@ -63,7 +63,7 @@ type PostgresqlEngineConfigurationReconciler struct { //nolint: golint,revive // // // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.12.1/pkg/reconcile -func (r *PostgresqlEngineConfigurationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { +func (r *PostgresqlEngineConfigurationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint:wsl // it is like that // Issue with this logger: controller and controllerKind are incorrect // Build another logger from upper to fix this. // reqLogger := log.FromContext(ctx) @@ -73,6 +73,7 @@ func (r *PostgresqlEngineConfigurationReconciler) Reconcile(ctx context.Context, // Fetch the PostgresqlEngineConfiguration instance instance := &postgresqlv1alpha1.PostgresqlEngineConfiguration{} + err := r.Get(ctx, req.NamespacedName, instance) if err != nil { if errors.IsNotFound(err) { @@ -94,13 +95,14 @@ func (r *PostgresqlEngineConfigurationReconciler) Reconcile(ctx context.Context, // Check if wait linked resources deletion flag is enabled if instance.Spec.WaitLinkedResourcesDeletion { // Check if there are linked resource linked to this - existingDB, err := r.getAnyDatabaseLinked(ctx, instance) + existingDB, err := r.getAnyDatabaseLinked(ctx, instance) //nolint:govet // Shadow err if err != nil { return r.manageError(ctx, reqLogger, instance, originalPatch, err) } + if existingDB != nil { // Wait for children removal - err := fmt.Errorf("cannot remove resource because found database %s in namespace %s linked to this resource and wait for deletion flag is enabled", existingDB.Name, existingDB.Namespace) + err = fmt.Errorf("cannot remove resource because found database %s in namespace %s linked to this resource and wait for deletion flag is enabled", existingDB.Name, existingDB.Namespace) return r.manageError(ctx, reqLogger, instance, originalPatch, err) } @@ -128,12 +130,13 @@ func (r *PostgresqlEngineConfigurationReconciler) Reconcile(ctx context.Context, // Check if the reconcile loop wasn't recall just because of update status if instance.Status.Phase == postgresqlv1alpha1.EngineValidatedPhase && instance.Status.LastValidatedTime != "" { - dur, err := time.ParseDuration(instance.Spec.CheckInterval) + dur, err := time.ParseDuration(instance.Spec.CheckInterval) //nolint:govet // Shadow err if err != nil { return r.manageError(ctx, reqLogger, instance, originalPatch, errors.NewInternalError(err)) } now := time.Now() + lastValidatedTime, err := time.Parse(time.RFC3339, instance.Status.LastValidatedTime) if err != nil { return r.manageError(ctx, reqLogger, instance, originalPatch, errors.NewInternalError(err)) @@ -152,6 +155,7 @@ func (r *PostgresqlEngineConfigurationReconciler) Reconcile(ctx context.Context, if instance.Status.Hash == hash { // Not changed => Requeue newWaitDuration := now.Add(dur).Sub(now) + reqLogger.Info("Reconcile skipped because called before check interval and nothing has changed") return ctrl.Result{Requeue: true, RequeueAfter: newWaitDuration}, err @@ -201,6 +205,7 @@ func (r *PostgresqlEngineConfigurationReconciler) Reconcile(ctx context.Context, // Check that secret is valid user := string(secret.Data["user"]) password := string(secret.Data["password"]) + if user == "" || password == "" { return r.manageError( ctx, @@ -267,7 +272,7 @@ func (r *PostgresqlEngineConfigurationReconciler) updateInstance( } // Add default values here to be saved in reconcile loop in order to help people to debug. -func (r *PostgresqlEngineConfigurationReconciler) addDefaultValues(instance *postgresqlv1alpha1.PostgresqlEngineConfiguration) { +func (*PostgresqlEngineConfigurationReconciler) addDefaultValues(instance *postgresqlv1alpha1.PostgresqlEngineConfiguration) { // Check port if instance.Spec.Port == 0 { instance.Spec.Port = 5432 diff --git a/controllers/postgresql/postgresqluser_controller.go b/controllers/postgresql/postgresqluser_controller.go index 9d13723..5557ced 100644 --- a/controllers/postgresql/postgresqluser_controller.go +++ b/controllers/postgresql/postgresqluser_controller.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "reflect" + "strconv" "time" corev1 "k8s.io/api/core/v1" @@ -48,7 +49,7 @@ const ( ) // PostgresqlUserReconciler reconciles a PostgresqlUser object. -type PostgresqlUserReconciler struct { //nolint: golint,revive // generated +type PostgresqlUserReconciler struct { //nolint: golint // generated client.Client Scheme *runtime.Scheme Recorder record.EventRecorder @@ -69,7 +70,7 @@ type PostgresqlUserReconciler struct { //nolint: golint,revive // generated // // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.12.1/pkg/reconcile -func (r *PostgresqlUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { +func (r *PostgresqlUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint:wsl // it is like that // Issue with this logger: controller and controllerKind are incorrect // Build another logger from upper to fix this. // reqLogger := log.FromContext(ctx) @@ -80,6 +81,7 @@ func (r *PostgresqlUserReconciler) Reconcile(ctx context.Context, req ctrl.Reque // Fetch the PostgresqlUser instance instance := &postgresqlv1alpha1.PostgresqlUser{} + err := r.Get(ctx, req.NamespacedName, instance) if err != nil { if errors.IsNotFound(err) { @@ -198,6 +200,7 @@ func (r *PostgresqlUserReconciler) Reconcile(ctx context.Context, req ctrl.Reque // Grant group role to user role var groupRole string + switch instance.Spec.Privileges { case postgresqlv1alpha1.ReaderPrivilege: groupRole = pgDB.Status.Roles.Reader @@ -215,6 +218,7 @@ func (r *PostgresqlUserReconciler) Reconcile(ctx context.Context, req ctrl.Reque return r.manageError(ctx, reqLogger, instance, originalPatch, err) } } + err = pgInstance.GrantRole(groupRole, role) if err != nil { return r.manageError(ctx, reqLogger, instance, originalPatch, err) @@ -246,16 +250,17 @@ func (r *PostgresqlUserReconciler) Reconcile(ctx context.Context, req ctrl.Reque } // Check if error exists and if it a not found error - if err != nil && errors.IsNotFound(err) { + if err != nil && errors.IsNotFound(err) { //nolint:gocritic // Won't change to a switch // Secret wasn't already present - // Update password in pg err = pgInstance.UpdatePassword(role, password) if err != nil { return r.manageError(ctx, reqLogger, instance, originalPatch, err) } + reqLogger.Info("Creating secret", "Secret.Namespace", generatedSecret.Namespace, "Secret.Name", generatedSecret.Name) r.Recorder.Event(instance, "Normal", "Processing", fmt.Sprintf("Creating secret %s for Postgresql User", generatedSecret.Name)) + err = r.Create(ctx, generatedSecret) if err != nil { return r.manageError(ctx, reqLogger, instance, originalPatch, err) @@ -396,11 +401,11 @@ func (r *PostgresqlUserReconciler) updateInstance( return false, nil } -func (r *PostgresqlUserReconciler) manageCreateUserRole(reqLogger logr.Logger, pgInstance postgres.PG, instance *postgresqlv1alpha1.PostgresqlUser, password string) (string, string, error) { +func (*PostgresqlUserReconciler) manageCreateUserRole(_ logr.Logger, pgInstance postgres.PG, instance *postgresqlv1alpha1.PostgresqlUser, password string) (role string, login string, err error) { // Delete old role if exists if instance.Status.RolePrefix != "" { // Drop old role - err := pgInstance.DropRoleAndDropAndChangeOwnedBy( + err = pgInstance.DropRoleAndDropAndChangeOwnedBy( instance.Status.PostgresRole, instance.Status.PostgresGroup, instance.Status.PostgresDatabaseName, @@ -411,7 +416,7 @@ func (r *PostgresqlUserReconciler) manageCreateUserRole(reqLogger logr.Logger, p } // Create new role suffix := utils.GetRandomString(RoleSuffixSize) - role := fmt.Sprintf("%s-%s", instance.Spec.RolePrefix, suffix) + role = fmt.Sprintf("%s-%s", instance.Spec.RolePrefix, suffix) // Check role length if len(role) > postgres.MaxIdentifierLength { @@ -420,7 +425,7 @@ func (r *PostgresqlUserReconciler) manageCreateUserRole(reqLogger logr.Logger, p return "", "", errors.NewBadRequest(errStr) } - login, err := pgInstance.CreateUserRole(role, password) + login, err = pgInstance.CreateUserRole(role, password) if err != nil { return "", "", err } @@ -447,7 +452,7 @@ func (r *PostgresqlUserReconciler) updatePGUserSecret( return nil } -func (r *PostgresqlUserReconciler) isSecretValid(foundSecret, newSecret *corev1.Secret) bool { +func (*PostgresqlUserReconciler) isSecretValid(foundSecret, newSecret *corev1.Secret) bool { // Get data foundData := foundSecret.Data newData := newSecret.Data @@ -503,7 +508,7 @@ func (r *PostgresqlUserReconciler) newSecretForPGUser(instance *postgresqlv1alph "LOGIN": []byte(login), "DATABASE": []byte(instance.Status.PostgresDatabaseName), "HOST": []byte(pg.GetHost()), - "PORT": []byte(fmt.Sprintf("%d", pg.GetPort())), + "PORT": []byte(strconv.Itoa(pg.GetPort())), "ARGS": []byte(pg.GetArgs()), }, } diff --git a/controllers/postgresql/postgresqluserrole_controller.go b/controllers/postgresql/postgresqluserrole_controller.go index ed940d2..eefc5fe 100644 --- a/controllers/postgresql/postgresqluserrole_controller.go +++ b/controllers/postgresql/postgresqluserrole_controller.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "reflect" + "strconv" "strings" "time" @@ -55,7 +56,7 @@ const ( ) // PostgresqlUserRoleReconciler reconciles a PostgresqlUserRole object. -type PostgresqlUserRoleReconciler struct { //nolint:revive // Ignore change name +type PostgresqlUserRoleReconciler struct { client.Client Scheme *runtime.Scheme Recorder record.EventRecorder @@ -82,7 +83,7 @@ type dbPrivilegeCache struct { // // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.12.1/pkg/reconcile -func (r *PostgresqlUserRoleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { +func (r *PostgresqlUserRoleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { //nolint:wsl // it is like that // Issue with this logger: controller and controllerKind are incorrect // Build another logger from upper to fix this. // reqLogger := log.FromContext(ctx) @@ -94,6 +95,7 @@ func (r *PostgresqlUserRoleReconciler) Reconcile(ctx context.Context, req ctrl.R // Fetch the PostgresqlUser instance instance := &v1alpha1.PostgresqlUserRole{} err := r.Get(ctx, req.NamespacedName, instance) + if err != nil { if errors.IsNotFound(err) { // Request object not found, could have been deleted after reconcile request. @@ -109,7 +111,7 @@ func (r *PostgresqlUserRoleReconciler) Reconcile(ctx context.Context, req ctrl.R originalPatch := client.MergeFrom(instance.DeepCopy()) // Deletion case - if !instance.GetDeletionTimestamp().IsZero() { + if !instance.GetDeletionTimestamp().IsZero() { //nolint:wsl // it is like that // Deletion detected // Check status postgresrole and so if user have been created @@ -123,7 +125,7 @@ func (r *PostgresqlUserRoleReconciler) Reconcile(ctx context.Context, req ctrl.R // Get needed items // Find PG Database cache - dbCache, pgecDBPrivilegeCache, err := r.getDatabaseInstances(ctx, instance, true) + dbCache, pgecDBPrivilegeCache, err := r.getDatabaseInstances(ctx, instance, true) //nolint:govet // Allow err shadow // Check error if err != nil { return r.manageError(ctx, reqLogger, instance, originalPatch, err) @@ -154,6 +156,7 @@ func (r *PostgresqlUserRoleReconciler) Reconcile(ctx context.Context, req ctrl.R if err != nil { return r.manageError(ctx, reqLogger, instance, originalPatch, err) } + reqLogger.Info("Successfully deleted") // Stop reconcile return reconcile.Result{}, nil @@ -205,8 +208,11 @@ func (r *PostgresqlUserRoleReconciler) Reconcile(ctx context.Context, req ctrl.R } var usernameChanged, passwordChanged, rotateUserPasswordError bool + var workSec *corev1.Secret - oldUsername := "" + + var oldUsername string + // Check if it is a provided user if instance.Spec.Mode == v1alpha1.ProvidedMode { workSec, oldUsername, passwordChanged, err = r.createOrUpdateWorkSecretForProvidedMode(ctx, reqLogger, instance) @@ -276,15 +282,17 @@ func (r *PostgresqlUserRoleReconciler) Reconcile(ctx context.Context, req ctrl.R } // Create or update user role if necessary - err = r.managePGUserRoles(ctx, reqLogger, instance, pgInstancesCache, username, password, usernameChanged, passwordChanged) + err = r.managePGUserRoles(ctx, reqLogger, instance, pgInstancesCache, username, password, passwordChanged) // Check error if err != nil { return r.manageError(ctx, reqLogger, instance, originalPatch, err) } + // Save important status now // Note: This is important to have a chance to have old username for deletion instance.Status.PostgresRole = username instance.Status.RolePrefix = instance.Spec.RolePrefix + if passwordChanged || usernameChanged || instance.Status.LastPasswordChangedTime == "" { instance.Status.LastPasswordChangedTime = time.Now().Format(time.RFC3339) } @@ -406,7 +414,7 @@ func (r *PostgresqlUserRoleReconciler) manageSecrets( func (r *PostgresqlUserRoleReconciler) cleanOldSecrets( ctx context.Context, - logger logr.Logger, + _ logr.Logger, instance *v1alpha1.PostgresqlUserRole, pgecDBPrivilegeCache map[string][]*dbPrivilegeCache, ) error { @@ -497,7 +505,7 @@ func (r *PostgresqlUserRoleReconciler) newSecretForPGUser( "LOGIN": []byte(username), "DATABASE": []byte(dbInstance.Status.Database), "HOST": []byte(pg.GetHost()), - "PORT": []byte(fmt.Sprintf("%d", pg.GetPort())), + "PORT": []byte(strconv.Itoa(pg.GetPort())), "ARGS": []byte(pg.GetArgs()), }, } @@ -512,7 +520,7 @@ func (r *PostgresqlUserRoleReconciler) newSecretForPGUser( } func (r *PostgresqlUserRoleReconciler) managePGUserRights( - ctx context.Context, + _ context.Context, logger logr.Logger, instance *v1alpha1.PostgresqlUserRole, pgInstanceCache map[string]postgres.PG, @@ -551,6 +559,7 @@ func (r *PostgresqlUserRoleReconciler) managePGUserRights( if err != nil { return err } + logger.Info("Successfully granted user in engine", "postgresqlEngine", key, "groupRole", groupRole) r.Recorder.Eventf(instance, "Normal", "Updated", "Successfully granted user to %s in engine %s", groupRole, key) } else { @@ -563,13 +572,14 @@ func (r *PostgresqlUserRoleReconciler) managePGUserRights( return c.Database == pcache.DBInstance.Status.Database }) // Check if not found or if group role have changed - if found == nil || found.(*postgres.SetRoleOnDatabaseRoleSetting).Role != groupRole { + if found == nil || found.(*postgres.SetRoleOnDatabaseRoleSetting).Role != groupRole { //nolint:forcetypeassert//We know // Add alter err = pgInstance.AlterDefaultLoginRoleOnDatabase(username, groupRole, pcache.DBInstance.Status.Database) // Check error if err != nil { return err } + logger.Info( "Successfully altered default login role in engine on specific database", "postgresqlEngine", key, @@ -589,7 +599,7 @@ func (r *PostgresqlUserRoleReconciler) managePGUserRights( if found != nil { // Remove from list to keep only the deletion ones // TODO Check if can be better to use samber/lo instead of funk - setRoleSettings = funk.Subtract(setRoleSettings, []*postgres.SetRoleOnDatabaseRoleSetting{found.(*postgres.SetRoleOnDatabaseRoleSetting)}).([]*postgres.SetRoleOnDatabaseRoleSetting) + setRoleSettings, _ = funk.Subtract(setRoleSettings, []*postgres.SetRoleOnDatabaseRoleSetting{found.(*postgres.SetRoleOnDatabaseRoleSetting)}).([]*postgres.SetRoleOnDatabaseRoleSetting) } } @@ -601,6 +611,7 @@ func (r *PostgresqlUserRoleReconciler) managePGUserRights( if err != nil { return err } + logger.Info("Successfully revoked role from user in engine", "postgresqlEngine", key, "role", role) r.Recorder.Eventf(instance, "Normal", "Updated", "Successfully revoked role %s from user in engine %s", role, key) } @@ -612,6 +623,7 @@ func (r *PostgresqlUserRoleReconciler) managePGUserRights( if err != nil { return err } + logger.Info("Successfully revoked set role from user on specific database in engine", "postgresqlEngine", key, "role", item.Role, "database", item.Database) r.Recorder.Eventf(instance, "Normal", "Updated", "Successfully revoked set role %s from user on specific database %s in engine %s", item.Role, item.Database, key) } @@ -621,7 +633,7 @@ func (r *PostgresqlUserRoleReconciler) managePGUserRights( return nil } -func (r *PostgresqlUserRoleReconciler) getDBRoleFromPrivilege(dbInstance *v1alpha1.PostgresqlDatabase, userRolePrivilege *v1alpha1.PostgresqlUserRolePrivilege) string { +func (*PostgresqlUserRoleReconciler) getDBRoleFromPrivilege(dbInstance *v1alpha1.PostgresqlDatabase, userRolePrivilege *v1alpha1.PostgresqlUserRolePrivilege) string { switch userRolePrivilege.Privilege { case v1alpha1.ReaderPrivilege: return dbInstance.Status.Roles.Reader @@ -633,12 +645,12 @@ func (r *PostgresqlUserRoleReconciler) getDBRoleFromPrivilege(dbInstance *v1alph } func (r *PostgresqlUserRoleReconciler) managePGUserRoles( - ctx context.Context, + _ context.Context, logger logr.Logger, instance *v1alpha1.PostgresqlUserRole, pgInstanceCache map[string]postgres.PG, username, password string, - usernameChanged, passwordChanged bool, + passwordChanged bool, ) error { // Loop over all pg instances for key, pgInstance := range pgInstanceCache { @@ -656,6 +668,7 @@ func (r *PostgresqlUserRoleReconciler) managePGUserRoles( if err != nil { return err } + logger.Info("Successfully created user in engine", "postgresqlEngine", key) r.Recorder.Eventf(instance, "Normal", "Updated", "Successfully created user in engine %s", key) // Stop here @@ -671,6 +684,7 @@ func (r *PostgresqlUserRoleReconciler) managePGUserRoles( if err != nil { return err } + logger.Info("Successfully updated user password in engine", "postgresqlEngine", key) r.Recorder.Eventf(instance, "Normal", "Updated", "Successfully updated user password in engine %s", key) } @@ -680,7 +694,7 @@ func (r *PostgresqlUserRoleReconciler) managePGUserRoles( return nil } -func (r *PostgresqlUserRoleReconciler) createOrUpdateWorkSecretForManagedMode( +func (r *PostgresqlUserRoleReconciler) createOrUpdateWorkSecretForManagedMode( //nolint:revive // We have multiple return, we know ctx context.Context, logger logr.Logger, instance *v1alpha1.PostgresqlUserRole, @@ -700,7 +714,7 @@ func (r *PostgresqlUserRoleReconciler) createOrUpdateWorkSecretForManagedMode( } // Check if error exist and not found // or check is secret must be updated. - if err != nil && errors.IsNotFound(err) { + if err != nil && errors.IsNotFound(err) { //nolint:gocritic // Won't change to a switch // Check if we are in the init phase // If not, that case shouldn't happened and a password change must be ensured as we cannot compare with previous // Also we must compare with the username previously set to check if username have changed @@ -948,7 +962,7 @@ func (r *PostgresqlUserRoleReconciler) newWorkSecret(instance *v1alpha1.Postgres } func (r *PostgresqlUserRoleReconciler) manageActiveSessionsAndDropOldRoles( - ctx context.Context, + _ context.Context, logger logr.Logger, instance *v1alpha1.PostgresqlUserRole, pgInstanceCache map[string]postgres.PG, @@ -1194,13 +1208,16 @@ func (r *PostgresqlUserRoleReconciler) validateInstance( // Prepare values priviNamespace := privi.Database.Namespace privi2Namespace := privi2.Database.Namespace + // Populate with instance if priviNamespace == "" { priviNamespace = instance.Namespace } + if privi2Namespace == "" { privi2Namespace = instance.Namespace } + // Check if privi.Database.Name == privi2.Database.Name && priviNamespace == privi2Namespace { return errors.NewBadRequest("Privilege list mustn't have the same database listed multiple times") diff --git a/controllers/utils/utils.go b/controllers/utils/utils.go index 0f09163..a067d13 100644 --- a/controllers/utils/utils.go +++ b/controllers/utils/utils.go @@ -3,8 +3,8 @@ package utils import ( "context" "crypto/sha256" + "encoding/hex" "encoding/json" - "fmt" "github.com/easymile/postgresql-operator/apis/postgresql/common" postgresqlv1alpha1 "github.com/easymile/postgresql-operator/apis/postgresql/v1alpha1" @@ -25,7 +25,7 @@ func CalculateHash(spec interface{}) (string, error) { sha256Res := sha256.Sum256(bytes) sha256Bytes := sha256Res[:] // Transform it to string - return fmt.Sprintf("%x", sha256Bytes), nil + return hex.EncodeToString(sha256Bytes), nil } func CreatePgInstance( diff --git a/main.go b/main.go index ae74b6c..2296d06 100644 --- a/main.go +++ b/main.go @@ -48,19 +48,20 @@ func init() { utilruntime.Must(postgresqlv1alpha1.AddToScheme(scheme)) //+kubebuilder:scaffold:scheme -} +} //nolint: wsl // Needed by operator func main() { - var metricsAddr string + var metricsAddr, probeAddr, resyncPeriodStr string + var enableLeaderElection bool - var probeAddr string - var resyncPeriodStr string + flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") flag.StringVar(&resyncPeriodStr, "resync-period", "30s", "The resync period to reload all resources for auto-heal procedures.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. "+ "Enabling this will ensure there is only one active controller manager.") + opts := zap.Options{ Development: false, } @@ -120,6 +121,7 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "PostgresqlEngineConfiguration") os.Exit(1) } + if err = (&postgresqlcontrollers.PostgresqlDatabaseReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), @@ -136,6 +138,7 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "PostgresqlDatabase") os.Exit(1) } + if err = (&postgresqlcontrollers.PostgresqlUserReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), @@ -152,6 +155,7 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "PostgresqlUser") os.Exit(1) } + if err = (&postgresqlcontrollers.PostgresqlUserRoleReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), @@ -174,12 +178,14 @@ func main() { setupLog.Error(err, "unable to set up health check") os.Exit(1) } + if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil { setupLog.Error(err, "unable to set up ready check") os.Exit(1) } setupLog.Info("starting manager") + if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { setupLog.Error(err, "problem running manager") os.Exit(1)