mirror of
https://github.com/actions-runner-controller/actions-runner-controller.git
synced 2026-10-04 03:31:23 +02:00
Release AutoscalingRunnerSet deletion when the GitHub config secret is gone (#4688)
Signed-off-by: Ankit Jha <ankit.jha@tradomate.one>
This commit is contained in:
@@ -4,12 +4,14 @@ import (
|
||||
"context"
|
||||
"crypto/x509"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
|
||||
"github.com/Azure/azure-sdk-for-go/sdk/azcore"
|
||||
"github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1/appconfig"
|
||||
"github.com/actions/actions-runner-controller/controllers/actions.github.com/multiclient"
|
||||
"github.com/actions/actions-runner-controller/controllers/actions.github.com/object"
|
||||
@@ -17,10 +19,35 @@ import (
|
||||
"github.com/actions/actions-runner-controller/vault/azurekeyvault"
|
||||
"golang.org/x/net/http/httpproxy"
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
kerrors "k8s.io/apimachinery/pkg/api/errors"
|
||||
"k8s.io/apimachinery/pkg/types"
|
||||
"sigs.k8s.io/controller-runtime/pkg/client"
|
||||
)
|
||||
|
||||
type secretResolverError string
|
||||
|
||||
func (e secretResolverError) Error() string { return string(e) }
|
||||
|
||||
// ErrNotFound marks a secret or config map that no longer exists, whichever driver resolved it.
|
||||
const ErrNotFound = secretResolverError("not found")
|
||||
|
||||
// wrapKubernetesError tags a Kubernetes NotFound so callers need no knowledge of the driver.
|
||||
func wrapKubernetesError(err error) error {
|
||||
if kerrors.IsNotFound(err) {
|
||||
return fmt.Errorf("%w: %w", ErrNotFound, err)
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
// wrapAzureKeyVaultError tags only a missing Azure Key Vault secret, so a 403 or throttled read still requeues.
|
||||
func wrapAzureKeyVaultError(err error) error {
|
||||
var responseErr *azcore.ResponseError
|
||||
if errors.As(err, &responseErr) && responseErr.StatusCode == http.StatusNotFound && responseErr.ErrorCode == "SecretNotFound" {
|
||||
return fmt.Errorf("%w: %w", ErrNotFound, err)
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
type SecretResolver struct {
|
||||
k8sClient client.Client
|
||||
multiClient multiclient.MultiClient
|
||||
@@ -56,12 +83,12 @@ func New(k8sClient client.Client, scalesetMultiClient multiclient.MultiClient, o
|
||||
func (sr *SecretResolver) GetAppConfig(ctx context.Context, obj object.ActionsGitHubObject) (*appconfig.AppConfig, error) {
|
||||
resolver, err := sr.resolverForObject(ctx, obj)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to get resolver for object: %v", err)
|
||||
return nil, fmt.Errorf("failed to get resolver for object: %w", err)
|
||||
}
|
||||
|
||||
appConfig, err := resolver.appConfig(ctx, obj.GitHubConfigSecret())
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to resolve app config: %v", err)
|
||||
return nil, fmt.Errorf("failed to resolve app config: %w", err)
|
||||
}
|
||||
|
||||
return appConfig, nil
|
||||
@@ -70,12 +97,12 @@ func (sr *SecretResolver) GetAppConfig(ctx context.Context, obj object.ActionsGi
|
||||
func (sr *SecretResolver) GetActionsService(ctx context.Context, obj object.ActionsGitHubObject) (multiclient.Client, error) {
|
||||
resolver, err := sr.resolverForObject(ctx, obj)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to get resolver for object: %v", err)
|
||||
return nil, fmt.Errorf("failed to get resolver for object: %w", err)
|
||||
}
|
||||
|
||||
appConfig, err := resolver.appConfig(ctx, obj.GitHubConfigSecret())
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to resolve app config: %v", err)
|
||||
return nil, fmt.Errorf("failed to resolve app config: %w", err)
|
||||
}
|
||||
|
||||
var proxyFunc func(req *http.Request) (*url.URL, error)
|
||||
@@ -93,7 +120,7 @@ func (sr *SecretResolver) GetActionsService(ctx context.Context, obj object.Acti
|
||||
if ref := proxy.HTTP.CredentialSecretRef; ref != "" {
|
||||
u.User, err = resolver.proxyCredentials(ctx, ref)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to resolve proxy credentials: %v", err)
|
||||
return nil, fmt.Errorf("failed to resolve proxy credentials: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -109,7 +136,7 @@ func (sr *SecretResolver) GetActionsService(ctx context.Context, obj object.Acti
|
||||
if ref := proxy.HTTPS.CredentialSecretRef; ref != "" {
|
||||
u.User, err = resolver.proxyCredentials(ctx, ref)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to resolve proxy credentials: %v", err)
|
||||
return nil, fmt.Errorf("failed to resolve proxy credentials: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -134,7 +161,7 @@ func (sr *SecretResolver) GetActionsService(ctx context.Context, obj object.Acti
|
||||
&configmap,
|
||||
)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to get configmap %s: %w", name, err)
|
||||
return nil, fmt.Errorf("failed to get configmap %s: %w", name, wrapKubernetesError(err))
|
||||
}
|
||||
|
||||
return []byte(configmap.Data[key]), nil
|
||||
@@ -173,12 +200,12 @@ func (sr *SecretResolver) resolverForObject(ctx context.Context, obj object.Acti
|
||||
var secret corev1.Secret
|
||||
err := sr.k8sClient.Get(ctx, types.NamespacedName{Name: s, Namespace: obj.GetNamespace()}, &secret)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to get secret %s: %w", s, err)
|
||||
return nil, fmt.Errorf("failed to get secret %s: %w", s, wrapKubernetesError(err))
|
||||
}
|
||||
return &secret, nil
|
||||
})
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to create proxy config: %v", err)
|
||||
return nil, fmt.Errorf("failed to create proxy config: %w", err)
|
||||
}
|
||||
proxy = p
|
||||
}
|
||||
@@ -225,7 +252,7 @@ func (r *k8sResolver) appConfig(ctx context.Context, key string) (*appconfig.App
|
||||
nsName,
|
||||
secret,
|
||||
); err != nil {
|
||||
return nil, fmt.Errorf("failed to get kubernetes secret: %q", nsName.String())
|
||||
return nil, fmt.Errorf("failed to get kubernetes secret %q: %w", nsName.String(), wrapKubernetesError(err))
|
||||
}
|
||||
|
||||
return appconfig.FromSecret(secret)
|
||||
@@ -239,7 +266,7 @@ func (r *k8sResolver) proxyCredentials(ctx context.Context, key string) (*url.Us
|
||||
nsName,
|
||||
secret,
|
||||
); err != nil {
|
||||
return nil, fmt.Errorf("failed to get kubernetes secret: %q", nsName.String())
|
||||
return nil, fmt.Errorf("failed to get kubernetes secret %q: %w", nsName.String(), wrapKubernetesError(err))
|
||||
}
|
||||
|
||||
return url.UserPassword(
|
||||
@@ -255,7 +282,7 @@ type vaultResolver struct {
|
||||
func (r *vaultResolver) appConfig(ctx context.Context, key string) (*appconfig.AppConfig, error) {
|
||||
val, err := r.vault.GetSecret(ctx, key)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to resolve secret: %v", err)
|
||||
return nil, fmt.Errorf("failed to resolve secret: %w", wrapAzureKeyVaultError(err))
|
||||
}
|
||||
|
||||
return appconfig.FromJSONString(val)
|
||||
@@ -264,7 +291,7 @@ func (r *vaultResolver) appConfig(ctx context.Context, key string) (*appconfig.A
|
||||
func (r *vaultResolver) proxyCredentials(ctx context.Context, key string) (*url.Userinfo, error) {
|
||||
val, err := r.vault.GetSecret(ctx, key)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to resolve secret: %v", err)
|
||||
return nil, fmt.Errorf("failed to resolve secret: %w", wrapAzureKeyVaultError(err))
|
||||
}
|
||||
|
||||
type info struct {
|
||||
|
||||
@@ -0,0 +1,135 @@
|
||||
package secretresolver
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/Azure/azure-sdk-for-go/sdk/azcore"
|
||||
"github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1"
|
||||
"github.com/actions/actions-runner-controller/vault"
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
kerrors "k8s.io/apimachinery/pkg/api/errors"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apimachinery/pkg/runtime/schema"
|
||||
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
|
||||
"sigs.k8s.io/controller-runtime/pkg/client"
|
||||
"sigs.k8s.io/controller-runtime/pkg/client/fake"
|
||||
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
|
||||
)
|
||||
|
||||
type fakeVault struct{ err error }
|
||||
|
||||
func (v fakeVault) GetSecret(context.Context, string) (string, error) { return "", v.err }
|
||||
|
||||
func TestWrapKubernetesError(t *testing.T) {
|
||||
notFound := kerrors.NewNotFound(schema.GroupResource{Resource: "secrets"}, "github-config")
|
||||
if err := wrapKubernetesError(notFound); !errors.Is(err, ErrNotFound) || !kerrors.IsNotFound(err) {
|
||||
t.Fatalf("NotFound lost its identity: %v", err)
|
||||
}
|
||||
forbidden := kerrors.NewForbidden(schema.GroupResource{Resource: "secrets"}, "github-config", errors.New("denied"))
|
||||
if err := wrapKubernetesError(forbidden); errors.Is(err, ErrNotFound) {
|
||||
t.Fatalf("Forbidden reported as not found: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestVaultResolverTagsOnlySecretNotFound(t *testing.T) {
|
||||
tests := map[string]struct {
|
||||
err error
|
||||
notFound bool
|
||||
}{
|
||||
"404": {&azcore.ResponseError{StatusCode: http.StatusNotFound, ErrorCode: "SecretNotFound"}, true},
|
||||
"404 wrapped": {errors.Join(errors.New("failed to get secret"), &azcore.ResponseError{StatusCode: http.StatusNotFound, ErrorCode: "SecretNotFound"}), true},
|
||||
"404 other": {&azcore.ResponseError{StatusCode: http.StatusNotFound, ErrorCode: "VaultNotFound"}, false},
|
||||
"403": {&azcore.ResponseError{StatusCode: http.StatusForbidden, ErrorCode: "Forbidden"}, false},
|
||||
"network error": {errors.New("dial tcp: i/o timeout"), false},
|
||||
}
|
||||
for name, tt := range tests {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
r := &vaultResolver{vault: fakeVault{err: tt.err}}
|
||||
_, appErr := r.appConfig(context.Background(), "github-config")
|
||||
_, proxyErr := r.proxyCredentials(context.Background(), "proxy")
|
||||
for _, err := range []error{appErr, proxyErr} {
|
||||
if err == nil {
|
||||
t.Fatal("expected an error")
|
||||
}
|
||||
if got := errors.Is(err, ErrNotFound); got != tt.notFound {
|
||||
t.Fatalf("errors.Is(err, ErrNotFound) = %v, want %v: %v", got, tt.notFound, err)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetActionsServiceTagsEveryMissingDependency(t *testing.T) {
|
||||
const ns = "arc-runners"
|
||||
configSecret := &corev1.Secret{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: "github-config", Namespace: ns},
|
||||
Data: map[string][]byte{"github_token": []byte("token")},
|
||||
}
|
||||
proxyWithCredentials := &v1alpha1.ProxyConfig{
|
||||
HTTP: &v1alpha1.ProxyServerConfig{Url: "http://proxy.example.com:3128", CredentialSecretRef: "proxy-credentials"},
|
||||
}
|
||||
forbidden := interceptor.Funcs{
|
||||
Get: func(ctx context.Context, c client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
|
||||
return kerrors.NewForbidden(schema.GroupResource{Resource: "secrets"}, key.Name, errors.New("denied"))
|
||||
},
|
||||
}
|
||||
|
||||
tests := map[string]struct {
|
||||
spec v1alpha1.AutoscalingRunnerSetSpec
|
||||
objects []client.Object
|
||||
interceptor *interceptor.Funcs
|
||||
notFound bool
|
||||
}{
|
||||
"github config secret missing": {
|
||||
spec: v1alpha1.AutoscalingRunnerSetSpec{GitHubConfigSecret: "github-config"},
|
||||
notFound: true,
|
||||
},
|
||||
"proxy credential secret missing": {
|
||||
spec: v1alpha1.AutoscalingRunnerSetSpec{GitHubConfigSecret: "github-config", Proxy: proxyWithCredentials},
|
||||
objects: []client.Object{configSecret},
|
||||
notFound: true,
|
||||
},
|
||||
"tls config map missing": {
|
||||
spec: v1alpha1.AutoscalingRunnerSetSpec{
|
||||
GitHubConfigSecret: "github-config",
|
||||
GitHubServerTLS: &v1alpha1.TLSConfig{CertificateFrom: &v1alpha1.TLSCertificateSource{
|
||||
ConfigMapKeyRef: &corev1.ConfigMapKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "ca"}, Key: "ca.crt"},
|
||||
}},
|
||||
},
|
||||
objects: []client.Object{configSecret},
|
||||
notFound: true,
|
||||
},
|
||||
"vault proxy credential secret missing": {
|
||||
spec: v1alpha1.AutoscalingRunnerSetSpec{
|
||||
GitHubConfigSecret: "github-config",
|
||||
VaultConfig: &v1alpha1.VaultConfig{Type: vault.VaultTypeAzureKeyVault, Proxy: proxyWithCredentials},
|
||||
},
|
||||
notFound: true,
|
||||
},
|
||||
"forbidden is not missing": {
|
||||
spec: v1alpha1.AutoscalingRunnerSetSpec{GitHubConfigSecret: "github-config"},
|
||||
interceptor: &forbidden,
|
||||
notFound: false,
|
||||
},
|
||||
}
|
||||
for name, tt := range tests {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
builder := fake.NewClientBuilder().WithScheme(clientgoscheme.Scheme).WithObjects(tt.objects...)
|
||||
if tt.interceptor != nil {
|
||||
builder = builder.WithInterceptorFuncs(*tt.interceptor)
|
||||
}
|
||||
ars := &v1alpha1.AutoscalingRunnerSet{ObjectMeta: metav1.ObjectMeta{Name: "ars", Namespace: ns}, Spec: tt.spec}
|
||||
|
||||
_, err := New(builder.Build(), nil).GetActionsService(context.Background(), ars)
|
||||
if err == nil {
|
||||
t.Fatal("expected an error")
|
||||
}
|
||||
if got := errors.Is(err, ErrNotFound); got != tt.notFound {
|
||||
t.Fatalf("errors.Is(err, ErrNotFound) = %v, want %v: %v", got, tt.notFound, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user