partial code review comments

Signed-off-by: Tom Proctor <tomhjp@users.noreply.github.com>
This commit is contained in:
Tom Proctor 2024-10-04 20:23:32 +01:00
parent 048214f598
commit e1d2b459b1
4 changed files with 27 additions and 14 deletions

View File

@ -471,6 +471,7 @@ func runReconcilers(opts reconcilerOpts) {
Complete(&ProxyGroupReconciler{
recorder: eventRecorder,
tsNamespace: opts.tailscaleNamespace,
proxyImage: opts.proxyImage,
Client: mgr.GetClient(),
l: opts.log.Named("proxygroup-reconciler"),
clock: tstime.DefaultClock{},

View File

@ -56,6 +56,7 @@ type ProxyGroupReconciler struct {
recorder record.EventRecorder
clock tstime.Clock
tsNamespace string
proxyImage string
tsClient tsClient
mu sync.Mutex // protects following
@ -134,8 +135,8 @@ func (r *ProxyGroupReconciler) Reconcile(ctx context.Context, req reconcile.Requ
}
if err = r.maybeProvision(ctx, pg); err != nil {
logger.Errorf("error creating ProxyGroup resources: %w", err)
message := fmt.Sprintf("failed creating ProxyGroup: %s", err)
logger.Errorf("error provisioning ProxyGroup resources: %w", err)
message := fmt.Sprintf("failed provisioning ProxyGroup: %s", err)
r.recorder.Eventf(pg, corev1.EventTypeWarning, reasonProxyGroupCreationFailed, message)
return setStatusReady(pg, metav1.ConditionFalse, reasonProxyGroupCreationFailed, message)
}
@ -178,7 +179,7 @@ func (r *ProxyGroupReconciler) maybeProvision(ctx context.Context, pg *tsapi.Pro
cfgHash, err := r.ensureConfigSecretsCreated(ctx, pg, proxyClass)
if err != nil {
return fmt.Errorf("error creating secrets: %w", err)
return fmt.Errorf("error provisioning config Secrets: %w", err)
}
// State secrets are precreated so we can use the ProxyGroup CR as their owner ref.
stateSecrets := pgStateSecrets(pg, r.tsNamespace)
@ -188,7 +189,7 @@ func (r *ProxyGroupReconciler) maybeProvision(ctx context.Context, pg *tsapi.Pro
s.ObjectMeta.Annotations = sec.ObjectMeta.Annotations
s.ObjectMeta.OwnerReferences = sec.ObjectMeta.OwnerReferences
}); err != nil {
return fmt.Errorf("error creating state Secret: %w", err)
return fmt.Errorf("error provisioning state Secrets: %w", err)
}
}
sa := pgServiceAccount(pg, r.tsNamespace)
@ -197,7 +198,7 @@ func (r *ProxyGroupReconciler) maybeProvision(ctx context.Context, pg *tsapi.Pro
s.ObjectMeta.Annotations = sa.ObjectMeta.Annotations
s.ObjectMeta.OwnerReferences = sa.ObjectMeta.OwnerReferences
}); err != nil {
return fmt.Errorf("error creating ServiceAccount: %w", err)
return fmt.Errorf("error provisioning ServiceAccount: %w", err)
}
role := pgRole(pg, r.tsNamespace)
if _, err := createOrUpdate(ctx, r.Client, r.tsNamespace, role, func(r *rbacv1.Role) {
@ -206,7 +207,7 @@ func (r *ProxyGroupReconciler) maybeProvision(ctx context.Context, pg *tsapi.Pro
r.ObjectMeta.OwnerReferences = role.ObjectMeta.OwnerReferences
r.Rules = role.Rules
}); err != nil {
return fmt.Errorf("error creating Role: %w", err)
return fmt.Errorf("error provisioning Role: %w", err)
}
roleBinding := pgRoleBinding(pg, r.tsNamespace)
if _, err := createOrUpdate(ctx, r.Client, r.tsNamespace, roleBinding, func(r *rbacv1.RoleBinding) {
@ -216,9 +217,9 @@ func (r *ProxyGroupReconciler) maybeProvision(ctx context.Context, pg *tsapi.Pro
r.RoleRef = roleBinding.RoleRef
r.Subjects = roleBinding.Subjects
}); err != nil {
return fmt.Errorf("error creating RoleBinding: %w", err)
return fmt.Errorf("error provisioning RoleBinding: %w", err)
}
ss := pgStatefulSet(pg, r.tsNamespace, cfgHash)
ss := pgStatefulSet(pg, r.tsNamespace, r.proxyImage, cfgHash)
ss = applyProxyClassToStatefulSet(proxyClass, ss, nil, logger)
if _, err := createOrUpdate(ctx, r.Client, r.tsNamespace, ss, func(s *appsv1.StatefulSet) {
s.ObjectMeta.Labels = ss.ObjectMeta.Labels
@ -226,7 +227,7 @@ func (r *ProxyGroupReconciler) maybeProvision(ctx context.Context, pg *tsapi.Pro
s.ObjectMeta.OwnerReferences = ss.ObjectMeta.OwnerReferences
s.Spec = ss.Spec
}); err != nil {
return fmt.Errorf("error creating StatefulSet: %w", err)
return fmt.Errorf("error provisioning StatefulSet: %w", err)
}
if err := r.cleanupDanglingResources(ctx, pg); err != nil {
@ -410,9 +411,17 @@ func pgTailscaledConfig(pg *tsapi.ProxyGroup, class *tsapi.ProxyClass, idx int32
conf.AcceptRoutes = "true"
}
deviceAuthed := false
for _, d := range pg.Status.Devices {
if d.Hostname == *conf.Hostname {
deviceAuthed = true
break
}
}
if authKey != "" {
conf.AuthKey = &authKey
} else if shouldRetainAuthKey(oldSecret) {
} else if !deviceAuthed {
key, err := authKeyFromSecret(oldSecret)
if err != nil {
return nil, fmt.Errorf("error retrieving auth key from Secret: %w", err)

View File

@ -20,7 +20,7 @@
// Returns the base StatefulSet definition for a ProxyGroup. A ProxyClass may be
// applied over the top after.
func pgStatefulSet(pg *tsapi.ProxyGroup, namespace, cfgHash string) *appsv1.StatefulSet {
func pgStatefulSet(pg *tsapi.ProxyGroup, namespace, image, cfgHash string) *appsv1.StatefulSet {
return &appsv1.StatefulSet{
ObjectMeta: metav1.ObjectMeta{
Name: pg.Name,
@ -48,7 +48,7 @@ func pgStatefulSet(pg *tsapi.ProxyGroup, namespace, cfgHash string) *appsv1.Stat
InitContainers: []corev1.Container{
{
Name: "sysctler",
Image: fmt.Sprintf("tailscale/tailscale:%s", selfVersionImageTag()),
Image: image,
SecurityContext: &corev1.SecurityContext{
Privileged: ptr.To(true),
},
@ -64,7 +64,7 @@ func pgStatefulSet(pg *tsapi.ProxyGroup, namespace, cfgHash string) *appsv1.Stat
Containers: []corev1.Container{
{
Name: "tailscale",
Image: fmt.Sprintf("tailscale/tailscale:%s", selfVersionImageTag()),
Image: image,
Env: pgEnv(pg),
SecurityContext: &corev1.SecurityContext{
Capabilities: &corev1.Capabilities{

View File

@ -26,6 +26,8 @@
"tailscale.com/types/ptr"
)
const testProxyImage = "tailscale/tailscale:test"
func TestProxyGroup(t *testing.T) {
pg := &tsapi.ProxyGroup{
ObjectMeta: metav1.ObjectMeta{
@ -45,6 +47,7 @@ func TestProxyGroup(t *testing.T) {
cl := tstest.NewClock(tstest.ClockOpts{})
reconciler := &ProxyGroupReconciler{
tsNamespace: tsNamespace,
proxyImage: testProxyImage,
Client: fc,
tsClient: tsClient,
recorder: fr,
@ -141,7 +144,7 @@ func expectProxyGroupResources(t *testing.T, fc client.WithWatch, pg *tsapi.Prox
role := pgRole(pg, tsNamespace)
roleBinding := pgRoleBinding(pg, tsNamespace)
serviceAccount := pgServiceAccount(pg, tsNamespace)
statefulSet := pgStatefulSet(pg, tsNamespace, "")
statefulSet := pgStatefulSet(pg, tsNamespace, testProxyImage, "")
if shouldExist {
expectEqual(t, fc, role, nil)