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

Create the initial version of the OpenCTI charm #2

Merged
merged 35 commits into from
Jan 13, 2025
Merged

Conversation

weiiwang01
Copy link
Collaborator

Overview

Create the initial version of the OpenCTI charm, designed to run the OpenCTI platform and its workers.

Screenshot of the OpenCTI dashboard:
Screenshot from 2024-12-09 11-28-41

Rationale

Juju Events Changes

Module Changes

Library Changes

Checklist

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

license-eye has checked 53 files.

Valid Invalid Ignored Fixed
13 9 31 0
Click to see the invalid file list
  • lib/charms/data_platform_libs/v0/data_interfaces.py
  • lib/charms/data_platform_libs/v0/s3.py
  • lib/charms/grafana_k8s/v0/grafana_dashboard.py
  • lib/charms/loki_k8s/v1/loki_push_api.py
  • lib/charms/observability_libs/v0/juju_topology.py
  • lib/charms/prometheus_k8s/v0/prometheus_scrape.py
  • lib/charms/rabbitmq_k8s/v0/rabbitmq.py
  • lib/charms/redis_k8s/v0/redis.py
  • opencti_rock/rockcraft.yaml
Use this command to fix any missing license headers
```bash

docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix

</details>

lib/charms/data_platform_libs/v0/s3.py Show resolved Hide resolved
lib/charms/loki_k8s/v1/loki_push_api.py Show resolved Hide resolved
lib/charms/rabbitmq_k8s/v0/rabbitmq.py Show resolved Hide resolved
lib/charms/redis_k8s/v0/redis.py Show resolved Hide resolved
opencti_rock/rockcraft.yaml Show resolved Hide resolved
charmcraft.yaml Show resolved Hide resolved
opencti_rock/rockcraft.yaml Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated

Learn more about config at https://juju.is/docs/sdk/config
def _install_callback_script(self, health_check_url: str) -> pathlib.Path:

Choose a reason for hiding this comment

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

I think it might be more efficient if we have a review call for this, could you organise something with me and @gregory-schiano please

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, no problem, created.

@weiiwang01 weiiwang01 requested a review from a team as a code owner December 12, 2024 08:32
opencti_rock/rockcraft.yaml Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
Copy link

@yhaliaw yhaliaw left a comment

Choose a reason for hiding this comment

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

Reviewed up to src/charm.py.

@@ -8,5 +8,14 @@ jobs:
uses: canonical/operator-workflows/.github/workflows/test.yaml@main
secrets: inherit
with:
self-hosted-runner: true
self-hosted-runner-label: "edge"
self-hosted-runner: false
Copy link

Choose a reason for hiding this comment

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

For our team, I think we should generally use self-hosted runners on "edge".

On the other hand, I think "edge" does have more wait time than GitHub runner under Canonical org.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, the wait time for self-hosted runners is too long. I am currently rapidly developing the charm and will switch to the self-hosted runner once the charm is more stable. Thanks!

juju-channel: 3.6/stable
microk8s-addons: "dns ingress rbac storage"
pre-run-script: tests/integration/prepare.sh
self-hosted-runner: false
Copy link

Choose a reason for hiding this comment

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

See the self-hosted runner comment above.

self-hosted-runner: true
self-hosted-runner-label: "edge"
self-hosted-runner: false
integration-tests:
Copy link

Choose a reason for hiding this comment

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

Why is the test.yaml and integration_test.yaml combined?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It saves one workflow file and makes it easier to check CI failures since unit tests and integration tests are in the same job tab.

.trivyignore Outdated
Copy link

Choose a reason for hiding this comment

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

A lot of linting of "not present anymore, can be safely removed.".
I think we should check to see if these are needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I think most of them are false alarms. Maybe we should upgrade the workflow. Thanks!

charmcraft.yaml Outdated
title: OpenCTI Charm
summary: OpenCTI charm.
links:
documentation: https://discourse.charmhub.io
Copy link

Choose a reason for hiding this comment

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

I think this should be updated to a valid discourse topic.

On the content-cache-operator, not updating this line to a valid topic in the initial commit has caused the issues with upload-docs workflow in future PRs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, you are right, changed to point to the README for now until we have the documentation page

src/charm.py Outdated

https://discourse.charmhub.io/t/4208
"""
"""OpenCTI charm the service."""
Copy link

Choose a reason for hiding this comment

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

I think the wording here is a bit weird.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed to OpenCTI charm thanks!

src/charm.py Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
.trivyignore Outdated Show resolved Hide resolved
opencti_rock/rockcraft.yaml Outdated Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
redis_url = self._redis.url
# bug in the Redis library produces an ill-formed redis_url
# when the integration is not ready
if not redis_url or redis_url == "redis://None:None":

Choose a reason for hiding this comment

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

When does redis://None:None happen? Perhaps this is a bug in the charm? It is weird that it would be passing a URL that doesn't work

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is an issue in the Redis charm library. Perhaps we could contribute to the library and fix it? It should probably just return None in that situation.

Choose a reason for hiding this comment

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

Would you like to open an issue on the redis charm?

src/charm.py Show resolved Hide resolved
charmcraft.yaml Show resolved Hide resolved
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

license-eye has checked 55 files.

Valid Invalid Ignored Fixed
0 14 41 0
Click to see the invalid file list
  • charmcraft.yaml
  • generate-src-docs.sh
  • opencti_rock/rockcraft.yaml
  • src/charm.py
  • tests/conftest.py
  • tests/integration/init.py
  • tests/integration/conftest.py
  • tests/integration/prepare.sh
  • tests/integration/test_charm.py
  • tests/unit/init.py
  • tests/unit/conftest.py
  • tests/unit/state.py
  • tests/unit/test_charm.py
  • tox.ini
Use this command to fix any missing license headers
```bash

docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix

</details>

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

license-eye has checked 55 files.

Valid Invalid Ignored Fixed
0 14 41 0
Click to see the invalid file list
  • charmcraft.yaml
  • generate-src-docs.sh
  • opencti_rock/rockcraft.yaml
  • src/charm.py
  • tests/conftest.py
  • tests/integration/init.py
  • tests/integration/conftest.py
  • tests/integration/prepare.sh
  • tests/integration/test_charm.py
  • tests/unit/init.py
  • tests/unit/conftest.py
  • tests/unit/state.py
  • tests/unit/test_charm.py
  • tox.ini
Use this command to fix any missing license headers
```bash

docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix

</details>

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

license-eye has checked 55 files.

Valid Invalid Ignored Fixed
0 14 41 0
Click to see the invalid file list
  • charmcraft.yaml
  • generate-src-docs.sh
  • opencti_rock/rockcraft.yaml
  • src/charm.py
  • tests/conftest.py
  • tests/integration/init.py
  • tests/integration/conftest.py
  • tests/integration/prepare.sh
  • tests/integration/test_charm.py
  • tests/unit/init.py
  • tests/unit/conftest.py
  • tests/unit/state.py
  • tests/unit/test_charm.py
  • tox.ini
Use this command to fix any missing license headers
```bash

docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix

</details>

Copy link

github-actions bot commented Jan 7, 2025

Test coverage for b4ab9cd

Name           Stmts   Miss Branch BrPart  Cover   Missing
----------------------------------------------------------
src/charm.py     260     24     56      4    89%   127, 202-214, 232-233, 449-450, 537-543, 661, 665, 670
----------------------------------------------------------
TOTAL            260     24     56      4    89%

Static code analysis report

Run started:2025-01-07 18:40:48.964701

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 567
  Total lines skipped (#nosec): 3
  Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
  Total issues (by severity):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
  Total issues (by confidence):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
Files skipped (0):

@weiiwang01 weiiwang01 requested a review from yhaliaw January 13, 2025 04:55
@weiiwang01 weiiwang01 merged commit 6720473 into main Jan 13, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants