Skip to content

feat(group-ib): migrate connector to manager-supported mode (#7282) - #7330

Open
Hugo Dupras (jabesq) wants to merge 9 commits into
masterfrom
feat/7282-group-ib-manager-supported
Open

feat(group-ib): migrate connector to manager-supported mode (#7282)#7330
Hugo Dupras (jabesq) wants to merge 9 commits into
masterfrom
feat/7282-group-ib-manager-supported

Conversation

@jabesq

Copy link
Copy Markdown
Member

Proposed changes

  • Migrate the Group-IB external-import connector to manager-supported mode (deployable with XTM Composer) using a structure-preserving "lite" approach: existing file layout, class names (ConfigConnector, CustomConnector, ExternalImportConnector), scheduling loop, and logging are unchanged.
  • Route all configuration through a validated Pydantic ConnectorSettings model (src/settings.py) exposed via to_helper_config(), replacing the legacy yaml.load + pycti.get_config_variable pattern.
  • Set manager_supported: true in __metadata__/connector_manifest.json (while preserving verified: true).
  • Generate the manager-supported config artifacts: __metadata__/connector_config_schema.json and __metadata__/CONNECTOR_CONFIG_DOC.md, and regenerate .env.sample / src/config.yml.sample to the flat single-underscore variable structure.
  • Flatten the deeply-nested ti_api config into single-underscore fields (the connectors-sdk settings loader only resolves two-level SECTION_FIELD env vars).
  • Add a backward-compatibility shim in GroupIBTIApiConfig (model_validator(mode="before")) that migrates legacy double-underscore env vars (e.g. TI_API__PROXY__IP, TI_API__COLLECTIONS__APT_THREAT__ENABLE) to the flattened fields, emitting a DeprecationWarning.
  • Add a second backward-compatibility path that flattens legacy nested config.yml sub-sections (proxy, extra_settings, and slash-keyed collections like apt/threat) onto the same fields, also emitting a DeprecationWarning; the new flattened value wins when both forms are present.
  • Simplify ConfigConnector: replace the eager attribute-generation shim with explicit @property accessors for the values actually consumed, and read the collection/extra-settings helpers directly from the settings model (preserving None-for-unknown behavior the cyberintegrations lib relies on).
  • Add pydantic and connectors-sdk to src/requirements.txt and a src/__init__.py re-exporting ConnectorSettings.
  • Add unit tests (12 total) covering settings validation, connector wiring, and both legacy-config migration paths (tests/test_main.py, tests/tests_connector/test_settings.py, plus tests/conftest.py and tests/test-requirements.txt).

Related issues

Checklist

  • I consider the submitted work as finished
  • I have signed my commits using GPG key.
  • I tested the code for its functionality using different use cases
  • I added/update the relevant documentation (either on github or on notion)
  • Where necessary I refactored code to improve the overall quality

Further comments

Backward compatibility. Env-var and nested-config renames are backward-compatible via two deprecation shims in GroupIBTIApiConfig (warn only), so no forced config migration is required. Legacy double-underscore env vars (TI_API__PROXY__IP, TI_API__COLLECTIONS__APT_THREAT__ENABLE, …) and legacy nested config.yml sub-sections (proxy, extra_settings, slash-keyed collections such as apt/threat) are automatically flattened onto the new single-underscore fields, each emitting a DeprecationWarning. When both the old and new forms are present, the new flattened value wins. This was verified end-to-end by loading the full old nested config.yml.

Breaking changes / behavior notes:

  • log_level is now validated against lowercase {debug, info, warn, warning, error}; non-standard casing/values that pycti previously tolerated will now fail validation.
  • update_existing_data now defaults to true when unset (the old effective default was disabled), so deployments that omitted it will start updating existing entities. Both sample files now set it explicitly.
  • Required keys (OPENCTI_URL/OPENCTI_TOKEN, TI_API_USERNAME/TI_API_TOKEN) now fail fast at startup if entirely absent.
  • Dropped dead/unused config: CONNECTOR_CONFIDENCE_LEVEL, CONNECTOR_MQ_*, and CONNECTOR_DOCKER_CONTAINER_NAME (pycti sources MQ settings from platform registration).

…on warnings (#7282)

The manager-supported migration flattened the deeply-nested ti_api config
into single-underscore fields, which renamed every TI_API__* env var. Add a
backward-compatibility shim on GroupIBTIApiConfig that maps legacy
double-underscore variables to the new fields and emits a DeprecationWarning,
so existing deployments keep working. The canonical variable wins when both
are set. Covered by unit tests.
Replace the eager _expose_settings_as_attributes/_unwrap shim, which
generated flat attributes for every settings field (including unused
opencti_* and connector id/name/scope/log_level), with explicit properties
for the values actually consumed and helpers that read the nested settings
directly. Removes dead attribute generation while preserving the historical
ConfigConnector interface (secret unwrapping, ISO-8601 duration, and
None-for-unknown collection/extra settings).
…nings (#7282)

The manager-supported migration flattened the nested ti_api config, which
also broke deployments still using a nested config.yml (proxy, extra_settings
and slash-keyed collections sub-sections were silently ignored). Extend the
GroupIBTIApiConfig migration validator to flatten those nested sub-sections
onto the canonical fields (in addition to the legacy double-underscore env
vars), emitting a DeprecationWarning per section. The flattened (canonical)
value takes precedence when both forms are provided. Covered by unit tests.
@github-actions

Copy link
Copy Markdown

🔴 Connector Linter errors detected

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request migrates the external-import/group-ib connector to manager-supported mode (deployable via XTM Composer) by moving configuration loading/validation to connectors-sdk + Pydantic settings, while preserving the connector’s existing runtime structure and behavior.

Changes:

  • Introduces a validated ConnectorSettings model (with backward-compatible migration for legacy ti_api env vars and nested config.yml shapes) and wires OpenCTIConnectorHelper to the generated helper config.
  • Updates runtime/config artifacts for manager support: manifest flag, generated config JSON schema + markdown docs, and flattened .env.sample / config.yml.sample.
  • Adds unit tests covering settings validation, connector wiring, and legacy-config migration paths.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
external-import/group-ib/src/settings.py Adds Pydantic/connectors-sdk settings models, including legacy config flattening and validation.
external-import/group-ib/src/config.py Refactors configuration access to come from ConnectorSettings and exposes compatibility properties/helpers.
external-import/group-ib/src/lib/external_import.py Instantiates OpenCTIConnectorHelper using the settings-derived helper config.
external-import/group-ib/src/requirements.txt Adds runtime dependencies for settings/manager-supported mode (Pydantic + connectors-sdk).
external-import/group-ib/src/init.py Re-exports ConnectorSettings for schema generation tooling.
external-import/group-ib/metadata/connector_manifest.json Sets manager_supported: true.
external-import/group-ib/metadata/connector_config_schema.json Adds generated manager-supported config schema.
external-import/group-ib/metadata/CONNECTOR_CONFIG_DOC.md Adds generated configuration documentation for Composer/users.
external-import/group-ib/.env.sample Updates env var layout to flattened single-underscore variables with new defaults.
external-import/group-ib/src/config.yml.sample Updates sample YAML to the flattened ti_api field structure.
external-import/group-ib/README.md Points users to the generated configuration documentation.
external-import/group-ib/tests/conftest.py Adds test-time import path wiring for src/.
external-import/group-ib/tests/test_main.py Adds wiring/smoke tests for settings -> helper -> connector initialization.
external-import/group-ib/tests/tests_connector/test_settings.py Adds settings validation + legacy migration unit tests.
external-import/group-ib/tests/test-requirements.txt Adds pytest + references main connector requirements for tests.
Suppressed comments (1)

external-import/group-ib/src/settings.py:138

  • This warning uses DeprecationWarning, which is ignored by default, so operators may never see the env-var migration guidance. Consider using UserWarning (or logging) so the deprecation message is visible by default.
            warnings.warn(
                f"Environment variable 'TI_API_{legacy_key.upper()}' is deprecated and "
                f"will be removed in a future release; use "
                f"'TI_API_{canonical_key.upper()}' instead.",
                DeprecationWarning,

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +105 to +109
warnings.warn(
f"Deprecated nested 'ti_api.{section}' configuration detected; "
f"use the flattened '{section}_*' settings instead.",
DeprecationWarning,
stacklevel=2,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hugo Dupras (@jabesq) Thanks for this huge migration

Tested on my side with both configuration files .env and config.yml

  • Each config is read properly when enable it
  • Thanks for adding tests

Worth to know

  • Following our discussion, the config.yml is now read properly
  • Config is now loaded and validated once at startup, and removing the  os.environ  scan and  yaml.load  is a net good reduction

LGTM :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

filigran team Item from the Filigran team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(group-ib): migrate connector to be "manager_supported"

4 participants