msteamsv2: inherit global proxy_url into partial http_config

[mihir-dixit2k27]

Title: msteamsv2: inherit global proxy_url into partial http_config

Fixes #5374

When a receiver's http_config is nil, the global config is copied in as before. When a receiver has a partial http_config but no proxy set, the global proxy settings are now merged in so they take effect.

Which user-facing changes does this PR introduce?

[BUGFIX] msteamsv2: Fix global proxy_url not being inherited when a receiver defines a partial http_config without an explicit proxy.

Description

Fixes a bug in msteamsv2_configs where a receiver-level http_config containing only partial settings (e.g. http_headers) would cause the global proxy_url to be silently ignored.

Previously, the config wiring used cmp.Or(msteamsv2.HTTPConfig, c.Global.HTTPConfig) which only falls back to the global config when the receiver's http_config is completely absent. If a user sets any field under http_config in the receiver, the global proxy is dropped entirely.

Config that triggers the bug:

global:
  http_config:
    proxy_url: "http://proxy.example.com:3128"

receivers:
  - name: teams
    msteamsv2_configs:
      - webhook_url: "<url>"
        http_config:
          http_headers:
            teams_channel_id:
              values:
                - my-channel-id

Implementation notes:

• If http_config is nil on the receiver, the global config is deep-copied in (same behaviour as before, but avoids shared-pointer mutation).
• If http_config is set but has no proxy, only the ProxyConfig field is inherited from global - the rest of the receiver's config is left untouched.
• If the receiver explicitly sets its own proxy_url, it is not overwritten.
• No new dependencies introduced.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR changes msteamsv2 HTTP config unmarshalling to copy global HTTP settings safely and to inherit only the proxy URL when a local config is partial. It also adds tests for missing, partial, and locally overridden proxy settings.

Changes

MSTeamsV2 Proxy Inheritance

Layer / File(s)	Summary
Conditional HTTPConfig/proxy inheritance logic config/config.go	Replaces the previous cmp.Or assignment with conditional logic that copies the global HTTPConfig when the receiver's is nil, or fills only the proxy settings from global when the receiver has a partial HTTPConfig lacking a proxy URL.
Tests for proxy inheritance and precedence config/config_test.go	Adds three tests covering inheritance from global proxy_url with no local http_config, inheritance with a partial local http_config, and precedence of a local proxy_url over the global one.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise, specific, and accurately summarizes the main change to msteamsv2 proxy inheritance.
Description check	✅ Passed	The description is mostly complete: it includes the issue link, release notes, bug context, behavior details, and implementation notes.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

config/config_test.go (2)

1834-1867: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Missing test coverage for partial proxy sub-fields other than proxy_url.

None of the three tests exercise a receiver-level http_config that sets no_proxy, proxy_from_environment, or proxy_connect_header without proxy_url. As flagged in config/config.go (lines 599-603), the current implementation overwrites the entire ProxyConfig struct when only proxy_url is missing, which would silently drop such locally-configured fields. A test covering this scenario would catch that regression.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@config/config_test.go` around lines 1834 - 1867, Add coverage for
receiver-level partial http_config cases in
TestMSTeamsV2GlobalProxyInheritedWhenPartialHTTPConfig or a new test around Load
and MSTeamsV2Configs so it verifies local proxy sub-fields like no_proxy,
proxy_from_environment, or proxy_connect_header are preserved when global
proxy_url is inherited. The issue is that the current proxy merging path in the
config loading logic overwrites ProxyConfig when proxy_url is absent; update the
config merge behavior so it only fills missing proxy_url and leaves any
explicitly set sub-fields intact, and validate this with assertions on
rc.HTTPConfig.ProxyConfig after Load.

---

1803-1832: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider using testify/require for assertions instead of raw if/t.Fatalf.

These new tests use manual if checks with t.Fatalf/t.Errorf rather than require. As per coding guidelines, use github.com/stretchr/testify/require instead of github.com/stretchr/testify/assert in Go tests — implying testify is the expected assertion library for tests in this repo.

Also applies to: 1836-1867, 1871-1899

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@config/config_test.go` around lines 1803 - 1832, The new MSTeams V2 proxy
inheritance tests are using manual if checks and t.Fatalf/t.Errorf instead of
the repo’s preferred testify require assertions. Update
TestMSTeamsV2GlobalProxyInheritedWhenNoLocalHTTPConfig and the related MSTeams
V2 test functions in this block to use github.com/stretchr/testify/require for
all nil/value assertions, replacing raw conditionals while keeping the same
expectations and test flow.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@config/config.go`:
- Around line 599-603: The current fallback in the msteamsv2 HTTP config logic
overwrites the entire ProxyConfig instead of only inheriting proxy_url. Update
the conditional in the msteamsv2 HTTPConfig handling so that when
ProxyConfig.ProxyURL is nil, only the ProxyURL field is copied from
c.Global.HTTPConfig.ProxyConfig and the existing local NoProxy,
ProxyFromEnvironment, and ProxyConnectHeader values are preserved. Use the
msteamsv2.HTTPConfig.ProxyConfig and c.Global.HTTPConfig.ProxyConfig symbols to
locate the change.

---

Nitpick comments:
In `@config/config_test.go`:
- Around line 1834-1867: Add coverage for receiver-level partial http_config
cases in TestMSTeamsV2GlobalProxyInheritedWhenPartialHTTPConfig or a new test
around Load and MSTeamsV2Configs so it verifies local proxy sub-fields like
no_proxy, proxy_from_environment, or proxy_connect_header are preserved when
global proxy_url is inherited. The issue is that the current proxy merging path
in the config loading logic overwrites ProxyConfig when proxy_url is absent;
update the config merge behavior so it only fills missing proxy_url and leaves
any explicitly set sub-fields intact, and validate this with assertions on
rc.HTTPConfig.ProxyConfig after Load.
- Around line 1803-1832: The new MSTeams V2 proxy inheritance tests are using
manual if checks and t.Fatalf/t.Errorf instead of the repo’s preferred testify
require assertions. Update
TestMSTeamsV2GlobalProxyInheritedWhenNoLocalHTTPConfig and the related MSTeams
V2 test functions in this block to use github.com/stretchr/testify/require for
all nil/value assertions, replacing raw conditionals while keeping the same
expectations and test flow.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9bc82477-712e-42fa-ba3e-028d0a74f027

📥 Commits

Reviewing files that changed from the base of the PR and between 963ed72 and 1694ae9.

📒 Files selected for processing (2)

• config/config.go
• config/config_test.go