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

fix(chart): add namespace selector to webhooks when in singleNamespace mode #1237

Merged
merged 2 commits into from
Mar 27, 2022
Merged

fix(chart): add namespace selector to webhooks when in singleNamespace mode #1237

merged 2 commits into from
Mar 27, 2022

Conversation

Meroje
Copy link
Contributor

@Meroje Meroje commented Mar 17, 2022

As we found out in #1223 there is an issue starting from ARC 0.22.0 where the admission webhooks are now used for pods controlled by RunnerDeployments. The absence of namespace selector results in denials for pod creation from webhooks for one organization being called for another where they don't have the authentication to request runner tokens.

Fixes #1223

@mumoshu
Copy link
Collaborator

mumoshu commented Mar 17, 2022

The absence of namespace selector results in denials for pod creation from webhooks for one organization being called for another

Note that this happens only when you deploy one instance of ARC per namespace 😃

@Meroje
Copy link
Contributor Author

Meroje commented Mar 17, 2022

btw did we miss something there? Do you recommend another way to handle many organizations without using a PAT?

@mumoshu
Copy link
Collaborator

mumoshu commented Mar 17, 2022

btw did we miss something there? Do you recommend another way to handle many organizations without using a PAT?

@Meroje You aren't missing anything! I just wanted to point out that you're affected only when you deploy one ARC per namespace, which isn't quite common (yet).

@toast-gear
Copy link
Collaborator

@Meroje could you provide your configuration, I'm struggling to get this fix to work, have you tested it works in your environment?

@Meroje
Copy link
Contributor Author

Meroje commented Mar 23, 2022

@Meroje could you provide your configuration, I'm struggling to get this fix to work, have you tested it works in your environment?

yes everything has been running fine since we implemented this change.

How we're doing this is by having an empty (no templates) chart declaring yours as dependency (to test this PR we unpacked the dependency and applied the patch)

# Chart.yaml
apiVersion: v2
name: actions-runner-controller
version: 0.1.0
dependencies:
- name: actions-runner-controller
  version: "~0.16.0"
  repository: "https://actions-runner-controller.github.io/actions-runner-controller"

then we create a release of this chart per organization in our GHES with corresponding values (which means we have X controllers in a single namespace each watching a different namespace)

# values.yaml
actions-runner-controller:
  githubEnterpriseServerURL: https://xxxxxxx
  githubWebhookServer:
    enabled: true
    ingress:
      enabled: true
      annotations:
        kubernetes.io/ingress.class: alb
        alb.ingress.kubernetes.io/group.name: actions-runner-webhook
        alb.ingress.kubernetes.io/listen-ports: '[{"HTTPS":443}]'
        alb.ingress.kubernetes.io/target-type: ip
    service:
      annotations:
        kubernetes.io/ingress.class: alb
      type: NodePort
  scope:
    singleNamespace: true

# org1.values.yaml
actions-runner-controller:
  leaderElectionId: org1-leader
  authSecret:
    name: org1-controller-manager
    create: true
    github_app_id: xx
    github_app_installation_id: xx
  githubWebhookServer:
    ingress:
      hosts:
        - host: xxxxx
          paths:
            - path: "/org1"
              pathType: ImplementationSpecific
  scope:
    watchNamespace: org1
# deploy.sh
helm upgrade \
    "${orgName}" \
    "charts/actions-runner-controller" \
    --create-namespace \
    --namespace action-runner-system \
    --install \
    -f "charts/actions-runner-controller/values.yaml" \
    -f "charts/actions-runner-controller/${orgName}.values.yaml" \
    --set-file actions-runner-controller.authSecret.github_app_private_key=github-app.pem \
    --set-string actions-runner-controller.scope.watchNamespace=${orgName} \
    --wait

we then have another chart responsible to deploy our RunnerDeployments in the watched namespaces.

@toast-gear
Copy link
Collaborator

LGTM my environment was messed up and needed rebuilding

@toast-gear toast-gear merged commit 1f8a23c into actions:master Mar 27, 2022
@Meroje Meroje deleted the fix/webhook-namespaceSelector branch March 27, 2022 12:01
README.md Show resolved Hide resolved
@toshke
Copy link

toshke commented Apr 5, 2022

Just had this issue, and upgrading to 0.17.2 have fixed it 👍

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.

Runner created but no pods linked on GHES on Canary from 13/03/2022
4 participants