Skip to content

Conversation

@allanhung
Copy link

Summary

  • Fix ExternalName service handling to return gracefully without error since they don't have pod selectors
  • Add standardized service name mappings for Argo Workflow, Spark, and GitHub Actions runner workloads
  • Add comprehensive test coverage for both fixes (5 new test cases)
  • Improve code documentation with detailed comments explaining the rationale

Changes

ExternalName Service Fix (4 API versions)

  • Modified ServiceIdentityToLabelsForWorkloadSelection in v1alpha3, v1beta1, v2alpha1, v2beta1
  • ExternalName services now return (nil, false, nil) instead of throwing "service has no selector" error
  • Added inline comments explaining why ExternalName services are handled specially

Standardized Owner Kind Mappings

  • Workflowargo-workflow: Consistent identity for Argo Workflow pods
  • SparkApplicationspark: Groups Spark driver and executor pods
  • RunnerDeploymentactions-runner: Consistent identity for GitHub Actions runners
  • Service owners now include API group to prevent naming conflicts

Test Coverage

Added 5 new test cases in serviceidresolver_test.go:

  1. TestServiceIdentityToPodLabelsForWorkloadSelection_ServiceKind_ExternalNameService
  2. TestResolvePodToServiceIdentity_WorkflowOwner
  3. TestResolvePodToServiceIdentity_SparkApplicationOwner
  4. TestResolvePodToServiceIdentity_RunnerDeploymentOwner
  5. TestResolvePodToServiceIdentity_ServiceOwner

All tests pass successfully.

Test plan

  • Run unit tests: go test ./shared/serviceidresolver/... -v
  • Verify ExternalName service handling test passes
  • Verify owner kind mapping tests pass
  • Code formatting with go fmt

🤖 Generated with Claude Code

…pings

This commit addresses two issues: (1) ExternalName services now return gracefully without error since they don't have pod selectors, and (2) specific workload types (Workflow, SparkApplication, RunnerDeployment) now get standardized service names for consistent policy application. Added comprehensive test coverage and documentation for both fixes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@github-actions
Copy link

CLA Assistant Lite bot:
Thank you for your submission -- we really appreciate it! Like many open-source projects, we ask that every contributor signs our Contributor License Agreement (CLA) before we can accept their contribution. This is to protect your rights, ours, and those of future users of this software.

Here is the information GitHub has on the contributors of this Pull Request (who have not yet signed this CLA):

- GitHub handle: allanhung
- Name: Allan Hung
- Email: [email protected]

To acknowledge that your information above is correct, that we may record it, and that you have read, understood, and agreed to this CLA, please sign the CLA by posting a Pull Request Comment below, containing the following exact text:


I have read and understood the CLA and hereby agree to its terms by making this Pull Request Comment.


You can retrigger this bot by commenting recheck in this Pull Request

@allanhung
Copy link
Author

Closing this PR as it should be created in my fork repository instead.

@allanhung allanhung closed this Nov 12, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Nov 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant