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

Enable TLS in filebeat #101

Merged

Conversation

VariableDeclared
Copy link
Contributor

@VariableDeclared VariableDeclared commented Nov 22, 2022

Add TLS Support to the filebeat charm

Depends on PR for elastic beats layer

Thanks,
Peter

Added client TLS to filebeat to enable pushing to Graylog.

TLS Client is only used to install the PKI CA and does not
request certificates of its own.
@VariableDeclared VariableDeclared changed the title Lma improvements/graylog tls Enable TLS in filebeat Nov 28, 2022
Copy link
Contributor

@agileshaw agileshaw left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Just one question about the build image change

@@ -15,7 +15,7 @@ parts:
bases:
- build-on:
- name: ubuntu
channel: "20.04"
channel: "22.04"
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that you've changed the build base to 22.04. Have you tested the charm's behavior and confirmed that it is working as intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I can change this back - it was a issue with my workstation I was working around :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually testing again, so it appears that the build is broken on 20.04, but working on 22.04

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please attach the output for the 20.04 build?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@VariableDeclared
Copy link
Contributor Author

hey @agileshaw any advice on the focal/jammy build issue?

@agileshaw
Copy link
Contributor

Hi @VariableDeclared . Apologize for the late reply.

This is a known issue with Jinja and MarkupSafe2 in focal. A workaround can be found here: juju/charm-tools#650

@VariableDeclared
Copy link
Contributor Author

hey @agileshaw had a look, but that fix is not merged - I cannot see a workaround there, what's your suggestion?

Adding the arguement is not yet working:

2022-12-06 17:13:53.433 :: 2022-12-06 17:13:52.940 :: usage: charm-build [-h] [-l LOG_LEVEL] [-f] [-o OUTPUT_DIR] [-d BUILD_DIR]
2022-12-06 17:13:53.433 :: 2022-12-06 17:13:52.940 ::                    [-C CACHE_DIR] [-s SERIES] [--hide-metrics]
2022-12-06 17:13:53.433 :: 2022-12-06 17:13:52.940 ::                    [--interface-service INTERFACE_SERVICE] [-i LAYER_INDEX]
2022-12-06 17:13:53.433 :: 2022-12-06 17:13:52.940 ::                    [--no-local-layers] [-n NAME] [--write-lock-file]
2022-12-06 17:13:53.433 :: 2022-12-06 17:13:52.940 ::                    [--use-lock-file-branches] [--ignore-lock-file] [-r] [-R]
2022-12-06 17:13:53.433 :: 2022-12-06 17:13:52.940 ::                    [-w WHEELHOUSE_OVERRIDES] [-W] [-v] [--debug] [-c]
2022-12-06 17:13:53.433 :: 2022-12-06 17:13:52.940 ::                    [--charm-file] [--binary-wheels]
2022-12-06 17:13:53.433 :: 2022-12-06 17:13:52.940 ::                    [--binary-wheels-from-source] [--use-python-from-snap]
2022-12-06 17:13:53.433 :: 2022-12-06 17:13:52.940 ::                    [--description]
2022-12-06 17:13:53.433 :: 2022-12-06 17:13:52.940 ::                    [charm]
2022-12-06 17:13:53.433 :: 2022-12-06 17:13:52.940 :: charm-build: error: unrecognized arguments: --upgrade-buildvenv-core-deps

@agileshaw
Copy link
Contributor

Since the issue only seems to appear on focal, I think we can use jammy as build base (so you don't need to change back the metadata.yaml). Please attach the log of a jammy-built charm and test its behavior with some simple bundle (e.g. https://git.launchpad.net/charm-graylog/tree/src/tests/functional/tests/bundles/base-graylog.yaml). If everything checks out, that's a +1 from my book.

I'm also adding @esunar as a reviewer.

@agileshaw agileshaw requested a review from esunar December 6, 2022 17:30
@lathiat
Copy link

lathiat commented Dec 9, 2022

The fix here may work:
https://discourse.charmhub.io/t/install-or-update-python-packages-before-packing-a-charm/5158
canonical/charmcraft#551

parts:
  charm:
    charm-python-packages:
      # https:/canonical/charmcraft/issues/551
      - setuptools

@VariableDeclared
Copy link
Contributor Author

hey @lathiat it appears at some point this arg was removed:

➜  filebeat-charm git:(lma-improvements/graylog-tls) ✗ charmcraft version
Bad charmcraft.yaml content:
- extra field 'charm-python-packages' not permitted in 'parts.charm' configuration
Full execution log: '/home/pjds/snap/charmcraft/common/cache/charmcraft/log/charmcraft-20221209-145322.062198.log'
➜  filebeat-charm git:(lma-improvements/graylog-tls) ✗ vi charmcraft.yaml
➜  filebeat-charm git:(lma-improvements/graylog-tls) ✗ charmcraft version
2.1.0

@VariableDeclared
Copy link
Contributor Author

Following up on my last comment - the issue is related to the charm plugin and reactive plugin. In this case we're using reactive which doesn't appear to support this arguement.

Comment on lines +10 to +22
subordinate: true
tags:
- filebeat
requires:
beats-host:
interface: juju-info
scope: container
logstash:
interface: elastic-beats
elasticsearch:
interface: elasticsearch
kafka:
interface: kafka
Copy link
Contributor Author

Choose a reason for hiding this comment

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

hey @esunar and @agileshaw I've pushed some changes which I hope will help with the issues seen. Seems that build no longer works on my workstation, I had to add these lines to have a successful build.

Copy link
Contributor

Choose a reason for hiding this comment

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

The changes looks good to me. Could you please attach the log from some simple bundle deployment? (e.g. https://git.launchpad.net/charm-graylog/tree/src/tests/functional/tests/bundles/base-graylog.yaml). I would like to see all if this jammy-built charm doesn't break such a deployment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hey sure, this is the bundle I am using today:

https://pastebin.canonical.com/p/kcddqf7WsQ/

Copy link
Contributor

Choose a reason for hiding this comment

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

And do you have a juju status (or something similar that shows deployment status) output of the deployed bundle?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@esunar esunar merged commit 4e3a715 into canonical:master Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants