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

Add single quotes around the credentials_json var #2712

Merged

Conversation

andrewkroh
Copy link
Member

@andrewkroh andrewkroh commented Feb 17, 2022

What does this PR do?

This handlebar variable contains a string and is required to be
passed as a string to the Beat. It must be properly quoted because
it contains JSON which in YAML will be interpretted as an object.

In general all handlebar variables that are strings should be single-quoted.

Given the configuration input of

vars:
  credentials_json: '{\"fake\":\"creds\"}'

Fleet was producing a policy containing

credentials_json:
  \"fake\":\"creds\": null

and now will produce

credentials_json: '{\"fake\":\"creds\"}'

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

@andrewkroh andrewkroh added bug Something isn't working, use only for issues Team:Security-External Integrations Integration:gcp Google Cloud Platform labels Feb 17, 2022
@elasticmachine
Copy link

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@elasticmachine
Copy link

elasticmachine commented Feb 17, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-02-17T19:46:47.002+0000

  • Duration: 25 min 32 sec

Test stats 🧪

Test Results
Failed 0
Passed 28
Skipped 0
Total 28

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

This handlebar variable contains a string and is required to be
passed as a string to the Beat. It must be properly quoted because
it contains JSON which in YAML will be interpretted as an object.

In general all handlebar variables that are strings should be single-quoted.
@andrewkroh andrewkroh force-pushed the gcp/bugfix/credential-json-quoting branch from 10674f9 to 1bb8592 Compare February 17, 2022 15:20
@andrewkroh
Copy link
Member Author

andrewkroh commented Feb 17, 2022

edit: I must have setup something wrong b/c I cannot reproduce this result. And I am getting the correct output for values containing newlines.

This still breaks (in an odd way) when the input value contains newlines.

Given

vars:
  credentials_json: |
    {
        "type": "service_account",
        "project_id": "foo",
        "private_key_id": "x",
        "private_key": "",
        "client_email": "[email protected]",
        "client_id": "0",
        "auth_uri": "https://accounts.google.com/o/oauth2/auth",
        "token_uri": "https://oauth2.googleapis.com/token",
        "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs",
        "client_x509_cert_url": "https://foo.bar/path"
    }

and a template of credentials_json: '{{credentials_json}}'

I expect

credentials_json: '{
    "type": "service_account",
    "project_id": "foo",
    "private_key_id": "x",
    "private_key": "",
    "client_email": "[email protected]",
    "client_id": "0",
    "auth_uri": "https://accounts.google.com/o/oauth2/auth",
    "token_uri": "https://oauth2.googleapis.com/token",
    "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs",
    "client_x509_cert_url": "https://foo.bar/path"
}'

but I observe a policy with

        credentials_json:
          type: service_account
          project_id: foo
          private_key_id: x
          private_key: ''
          client_email: [email protected]
          client_id: '0'
          auth_uri: 'https://accounts.google.com/o/oauth2/auth'
          token_uri: 'https://oauth2.googleapis.com/token'
          auth_provider_x509_cert_url: 'https://www.googleapis.com/oauth2/v1/certs'
          client_x509_cert_url: 'https://foo.bar/path'

What happened to the single-quotes around the handlebar variable and why is the string now an object?

@endorama
Copy link
Member

endorama commented Feb 17, 2022

I tested this:

{{#if credentials_json}}
credentials_json: >
  {{credentials_json}}
{{/if}}

Which gives:

credentials_json: |
    {"type":"service_account",...}

This is a valid string even if it is not enclosed in quotes (but should not matter). I think Fleet is parsing the input from handlebars as YAML and rendering it back, that would explain why > becomes |.
Using > for a field that should fold newlines seems more appropriate. I've yet to understand if this is then correctly interpreted by the underlying beat (metricbeat in my case)

@andrewkroh
Copy link
Member Author

I tried that with a value containing a newline and got an error.

credentials_json: >
  {{credentials_json}}
{
  "statusCode": 500,
  "error": "Internal Server Error",
  "message": "end of the stream or a document separator is expected at line 16, column 1:\n    }\n    ^"
}

The policy looked like this

POST /api/fleet/package_policies
{"name":"gcp-audit","description":"","namespace":"ep","policy_id":"631c4190-9012-11ec-84d1-7d60bedf0cd9","enabled":true,"output_id":"","inputs":[{"type":"gcp-pubsub","enabled":true,"streams":[{"id":"gcp-pubsub-gcp.audit","enabled":true,"data_stream":{"type":"logs","dataset":"gcp.audit"},"vars":{"preserve_original_event":{"value":false,"type":"bool"},"processors":{"value":null,"type":"yaml"},"subscription_create":{"value":false,"type":"bool"},"subscription_name":{"value":"subscription","type":"text"},"tags":{"value":["forwarded","gcp-audit"],"type":"text"},"topic":{"value":"topic","type":"text"}}}],"vars":{"alternative_host":{"value":"elastic-package-service_gcppubsub-emulator_1:8681","type":"text"},"credentials_file":{"value":null,"type":"text"},"credentials_json":{"value":"{\n    \"type\": \"service_account\",\n    \"project_id\": \"foo\",\n    \"private_key_id\": \"x\",\n    \"private_key\": \"\",\n    \"client_email\": \"[email protected]\",\n    \"client_id\": \"0\",\n    \"auth_uri\": \"https://accounts.google.com/o/oauth2/auth\",\n    \"token_uri\": \"https://oauth2.googleapis.com/token\",\n    \"auth_provider_x509_cert_url\": \"https://www.googleapis.com/oauth2/v1/certs\",\n    \"client_x509_cert_url\": \"https://foo.bar/path\"\n}\n","type":"text"},"project_id":{"value":"audit","type":"text"}}}],"package":{"name":"gcp","title":"Google Cloud Platform","version":"1.4.7"}}

@andrewkroh
Copy link
Member Author

andrewkroh commented Feb 17, 2022

The CI error from Filebeat 7.17.0 is

{"log.level":"info","@timestamp":"2022-02-17T17:57:30.846Z","log.logger":"centralmgmt.fleet","log.origin":{"file.name":"management/manager.go","file.line":271},"message":"Applying settings for filebeat.inputs","service.name":"filebeat","ecs.version":"1.6.0"}
{"log.level":"error","@timestamp":"2022-02-17T17:57:30.847Z","log.logger":"centralmgmt","log.origin":{"file.name":"cfgfile/list.go","file.line":99},"message":"Error creating runner from config: the field can't be converted to valid JSON accessing 'credentials_json'","service.name":"filebeat","ecs.version":"1.6.0"}
{"log.level":"error","@timestamp":"2022-02-17T17:57:30.847Z","log.logger":"centralmgmt.fleet","log.origin":{"file.name":"management/manager.go","file.line":307},"message":"1 error: Error creating runner from config: the field can't be converted to valid JSON accessing 'credentials_json'","service.name":"filebeat","ecs.version":"1.6.0"}

It passes locally for me using both 7.16.3 and 7.17.0.

@andrewkroh andrewkroh force-pushed the gcp/bugfix/credential-json-quoting branch from 761ae91 to 3d22d1a Compare February 17, 2022 19:46
@andrewkroh andrewkroh merged commit 0457b90 into elastic:main Feb 17, 2022
eyalkraft pushed a commit to build-security/integrations that referenced this pull request Mar 30, 2022
This handlebar variable contains a string and is required to be
passed as a string to the Beat. It must be properly quoted because
it contains JSON which in YAML will be interpretted as an object.

In general all handlebar variables that are strings should be single-quoted.

To test Fleet's handling of JSON string I used a variety of formats including
some that contain newlines, end in news, without newlines, and an invalid
credential format (but valid json). One issue is that single quotes are not
properly escaped Fleet when evaluating the handlebar template leading to
invalid YAML, but this use case should not involve any single-quotes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working, use only for issues Integration:gcp Google Cloud Platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants