Skip to content

Fix audit log parse error#2905

Merged
Ganeshrockz merged 4 commits intomainfrom
net-5513/fix-integer
Sep 7, 2023
Merged

Fix audit log parse error#2905
Ganeshrockz merged 4 commits intomainfrom
net-5513/fix-integer

Conversation

@Ganeshrockz
Copy link
Copy Markdown
Contributor

@Ganeshrockz Ganeshrockz commented Sep 6, 2023

Changes proposed in this PR:

  • Fixes a bug in parsing the following audit log helm configuration
server:
  auditLogs:
      enabled: true
      sinks:
        - name: My Sink
          type: file
          format: json
          path: /tmp/audit.json
          delivery_guarantee: best-effort
          rotate_duration: 24h
          rotate_max_files: 15
          rotate_bytes: 25165824

where rotate_max_files and rotate_bytes get passed as string inputs to the server configuration file. This PR makes sure to perform special handling for rotate_max_files and rotate_bytes where they don't get passed with quotes.

Generated config (Before the fix)

    {
      "audit": {
        "enabled": true,
        "sink": {
          "MySink": {
              "delivery_guarantee": "best-effort",
              "format": "json",
              "path": "/tmp/audit.json",
              "rotate_bytes": "12455355", // note the presence of braces here
              "rotate_duration": "24h",
              "rotate_max_files": "20",
              "type": "file"
          }
        }
      }
    }

Generated config (After the fix)

    {
      "audit": {
        "enabled": true,
        "sink": {
          "MySink": {
              "delivery_guarantee": "best-effort",
              "format": "json",
              "path": "/tmp/audit.json",
              "rotate_bytes": 12455355, // note the absence of braces here
              "rotate_duration": "24h",
              "rotate_max_files": 20,
              "type": "file"
          }
        }
      }
    }

How I've tested this PR:

  1. CI
  2. Verified manually that the server no longer crashes with this issue.

How I expect reviewers to test this PR:

👀

Checklist:

@Ganeshrockz Ganeshrockz added backport/0.49.x backport/1.1.x Backport to release/1.1.x branch backport/1.2.x This release branch is no longer active. labels Sep 6, 2023
Copy link
Copy Markdown
Contributor

@asheshvidyut asheshvidyut left a comment

Choose a reason for hiding this comment

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

@asheshvidyut
Copy link
Copy Markdown
Contributor

asheshvidyut commented Sep 6, 2023

Can we do this

 "{{ $element.name }}" : {{ $element | toJson }}

instead of https://github.com/hashicorp/consul-k8s/pull/2905/files#diff-46b4101175c7cfa3c393941b3563a31d9c7f1c49f8b709b9f87e9d97e2da181eR202-R212

audit-logging.json: |-
    {
      "audit": {
        "enabled": true,
        "sink": {
          {{- range $index, $element := .Values.server.auditLogs.sinks }}
          {{- if ne $index 0 }},{{end}}
          {{- $name := $element.name }}
          {{- $_ := unset $element "name" }}
          "{{ $name }}" : {{$element | toJson }}
          {{- end }}
        }
      }
    }

playground link

@david-yu david-yu linked an issue Sep 6, 2023 that may be closed by this pull request
@david-yu
Copy link
Copy Markdown
Contributor

david-yu commented Sep 6, 2023

Removing 0.49.x since we no longer release that branch. Thanks!

@Ganeshrockz Ganeshrockz merged commit b66543f into main Sep 7, 2023
@Ganeshrockz Ganeshrockz deleted the net-5513/fix-integer branch September 7, 2023 04:10
Ganeshrockz added a commit that referenced this pull request Sep 8, 2023
* Create 2905.txt

* Update server-config-configmap.yaml

* Update server-config-configmap.bats
Ganeshrockz added a commit that referenced this pull request Sep 11, 2023
* Create 2905.txt

* Update server-config-configmap.yaml

* Update server-config-configmap.bats

* Update server-config-configmap.bats
@Ganeshrockz Ganeshrockz added the consul-india PRs/Issues assigned to Consul India team label Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/1.1.x Backport to release/1.1.x branch backport/1.2.x This release branch is no longer active. consul-india PRs/Issues assigned to Consul India team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enabling audit logging causes configuration parse failure

3 participants