Skip to content

Commit 59bc5b8

Browse files
maelvlsmaelvls
authored
Merge pull request #560 from jetstack/venconn
Feedback from #552
2 parents 803b953 + eb4ab3d commit 59bc5b8

File tree

5 files changed

+158
-108
lines changed

5 files changed

+158
-108
lines changed

‎.github/workflows/release-master.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,14 @@ jobs:
7070
packages: write
7171
id-token: write
7272
steps:
73-
- uses: webfactory/[email protected]
74-
with:
75-
ssh-private-key: ${{ secrets.DEPLOY_KEY_READ_VENAFI_CONNECTION_LIB }}
7673
- name: Install Tools
7774
# Installing 'bash' because it's required by the 'cosign-installer' action
7875
# and 'coreutils' because the 'slsa-provenance-action' requires a version
7976
# of 'base64' that supports the -w flag.
8077
run: apk add --update make git jq rsync curl bash coreutils go
78+
- uses: webfactory/[email protected]
79+
with:
80+
ssh-private-key: ${{ secrets.DEPLOY_KEY_READ_VENAFI_CONNECTION_LIB }}
8181
- name: Adding github workspace as safe directory
8282
# See issue https://github.com/actions/checkout/issues/760
8383
run: git config --global --add safe.directory $GITHUB_WORKSPACE

‎pkg/agent/run.go

Lines changed: 8 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"fmt"
88
"io"
99
"io/ioutil"
10-
"log"
1110
"net/http"
1211
_ "net/http/pprof"
1312
"net/url"
@@ -18,18 +17,17 @@ import (
1817

1918
"github.com/cenkalti/backoff"
2019
"github.com/hashicorp/go-multierror"
21-
"github.com/jetstack/preflight/pkg/logs"
2220
json "github.com/json-iterator/go"
2321
"github.com/prometheus/client_golang/prometheus"
2422
"github.com/prometheus/client_golang/prometheus/promhttp"
2523
"github.com/spf13/cobra"
26-
"k8s.io/client-go/rest"
27-
"k8s.io/client-go/tools/clientcmd"
2824
"sigs.k8s.io/controller-runtime/pkg/manager"
2925

3026
"github.com/jetstack/preflight/api"
3127
"github.com/jetstack/preflight/pkg/client"
3228
"github.com/jetstack/preflight/pkg/datagatherer"
29+
"github.com/jetstack/preflight/pkg/kubeconfig"
30+
"github.com/jetstack/preflight/pkg/logs"
3331
"github.com/jetstack/preflight/pkg/version"
3432
)
3533

@@ -302,7 +300,7 @@ func getConfiguration() (Config, client.Client) {
302300
if venConnMode && InstallNS == "" {
303301
InstallNS, err = getInClusterNamespace()
304302
if err != nil {
305-
log.Fatalf("could not guess which namespace the agent is running in: %s", err)
303+
logs.Log.Fatalf("could not guess which namespace the agent is running in: %s", err)
306304
}
307305
}
308306
if venConnMode && VenConnNS == "" {
@@ -323,15 +321,15 @@ func getConfiguration() (Config, client.Client) {
323321
// the --venafi-connection mode of authentication doesn't need any
324322
// secrets (or any other information for that matter) to be loaded from
325323
// disk (using --credentials-path). Everything is passed as flags.
326-
log.Println("Venafi Connection mode was specified, using Venafi Connection authentication.")
324+
logs.Log.Println("Venafi Connection mode was specified, using Venafi Connection authentication.")
327325

328326
// The venafi-cloud.upload_path was initially meant to let users
329327
// configure HTTP proxies, but it has never been used since HTTP proxies
330328
// don't rewrite paths. Thus, we've disabled the ability to change this
331329
// value with the new --venafi-connection flag, and this field is simply
332330
// ignored.
333331
if config.VenafiCloud != nil && config.VenafiCloud.UploadPath != "" {
334-
log.Printf(`ignoring venafi-cloud.upload_path. In Venafi Connection mode, this field is not needed.`)
332+
logs.Log.Printf(`ignoring venafi-cloud.upload_path. In Venafi Connection mode, this field is not needed.`)
335333
}
336334

337335
// Regarding venafi-cloud.uploader_id, we found that it doesn't do
@@ -340,12 +338,12 @@ func getConfiguration() (Config, client.Client) {
340338
// set in the config file, and set it to an arbitrary value in the
341339
// client since it doesn't matter.
342340
if config.VenafiCloud.UploaderID != "" {
343-
log.Printf(`ignoring venafi-cloud.uploader_id. In Venafi Connection mode, this field is not needed.`)
341+
logs.Log.Printf(`ignoring venafi-cloud.uploader_id. In Venafi Connection mode, this field is not needed.`)
344342
}
345343

346-
cfg, err := loadRESTConfig("")
344+
cfg, err := kubeconfig.LoadRESTConfig("")
347345
if err != nil {
348-
log.Fatalf("failed to load kubeconfig: %v", err)
346+
logs.Log.Fatalf("failed to load kubeconfig: %v", err)
349347
}
350348

351349
preflightClient, err = client.NewVenConnClient(cfg, agentMetadata, InstallNS, VenConnName, VenConnNS, nil)
@@ -569,28 +567,3 @@ func getInClusterNamespace() (string, error) {
569567
}
570568
return string(namespace), nil
571569
}
572-
573-
func loadRESTConfig(path string) (*rest.Config, error) {
574-
switch path {
575-
// If the kubeconfig path is not provided, use the default loading rules
576-
// so we read the regular KUBECONFIG variable or create a non-interactive
577-
// client for agents running in cluster
578-
case "":
579-
loadingrules := clientcmd.NewDefaultClientConfigLoadingRules()
580-
cfg, err := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(
581-
loadingrules, &clientcmd.ConfigOverrides{}).ClientConfig()
582-
if err != nil {
583-
return nil, fmt.Errorf("failed to load kubeconfig: %w", err)
584-
}
585-
return cfg, nil
586-
// Otherwise use the explicitly named kubeconfig file.
587-
default:
588-
cfg, err := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(
589-
&clientcmd.ClientConfigLoadingRules{ExplicitPath: path},
590-
&clientcmd.ConfigOverrides{}).ClientConfig()
591-
if err != nil {
592-
return nil, fmt.Errorf("failed to load kubeconfig from %s: %w", path, err)
593-
}
594-
return cfg, nil
595-
}
596-
}

‎pkg/client/client_venconn_test.go

Lines changed: 107 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
"github.com/stretchr/testify/require"
1818
corev1 "k8s.io/api/core/v1"
1919
rbacv1 "k8s.io/api/rbac/v1"
20-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2120
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2221
"k8s.io/apimachinery/pkg/runtime"
2322
"k8s.io/apimachinery/pkg/types"
@@ -102,6 +101,67 @@ func TestVenConnClient_PostDataReadingsWithOptions(t *testing.T) {
102101
}))
103102
}
104103

104+
// Generated using:
105+
//
106+
// helm template ./deploy/charts/venafi-kubernetes-agent -n venafi --set venafiConnection.include=true --show-only templates/venafi-connection-rbac.yaml | grep -ivE '(helm|\/version)'
107+
const rbac = `
108+
apiVersion: v1
109+
kind: Namespace
110+
metadata:
111+
name: venafi
112+
---
113+
# Source: venafi-kubernetes-agent/templates/venafi-connection-rbac.yaml
114+
# The 'venafi-connection' service account is used by multiple
115+
# controllers. When configuring which resources a VenafiConnection
116+
# can access, the RBAC rules you create manually must point to this SA.
117+
apiVersion: v1
118+
kind: ServiceAccount
119+
metadata:
120+
name: venafi-connection
121+
namespace: "venafi"
122+
labels:
123+
app.kubernetes.io/name: "venafi-connection"
124+
app.kubernetes.io/instance: release-name
125+
---
126+
# Source: venafi-kubernetes-agent/templates/venafi-connection-rbac.yaml
127+
apiVersion: rbac.authorization.k8s.io/v1
128+
kind: ClusterRole
129+
metadata:
130+
name: venafi-connection-role
131+
labels:
132+
app.kubernetes.io/name: "venafi-connection"
133+
app.kubernetes.io/instance: release-name
134+
rules:
135+
- apiGroups: [ "" ]
136+
resources: [ "namespaces" ]
137+
verbs: [ "get", "list", "watch" ]
138+
139+
- apiGroups: [ "jetstack.io" ]
140+
resources: [ "venaficonnections" ]
141+
verbs: [ "get", "list", "watch" ]
142+
143+
- apiGroups: [ "jetstack.io" ]
144+
resources: [ "venaficonnections/status" ]
145+
verbs: [ "get", "patch" ]
146+
---
147+
# Source: venafi-kubernetes-agent/templates/venafi-connection-rbac.yaml
148+
apiVersion: rbac.authorization.k8s.io/v1
149+
kind: ClusterRoleBinding
150+
metadata:
151+
name: venafi-connection-rolebinding
152+
labels:
153+
app.kubernetes.io/name: "venafi-connection"
154+
app.kubernetes.io/instance: release-name
155+
roleRef:
156+
apiGroup: rbac.authorization.k8s.io
157+
kind: ClusterRole
158+
name: venafi-connection-role
159+
subjects:
160+
- kind: ServiceAccount
161+
name: venafi-connection
162+
namespace: "venafi"
163+
`
164+
105165
type testcase struct {
106166
given string
107167
expectErr string
@@ -144,48 +204,55 @@ func run(test testcase) func(t *testing.T) {
144204
// Apply the same RBAC as what you would get from the Venafi
145205
// Connection Helm chart, for example after running this:
146206
// helm template venafi-connection oci://registry.venafi.cloud/charts/venafi-connection --version v0.1.0 -n venafi --show-only templates/venafi-connection-rbac.yaml
147-
require.NoError(t, kclient.Create(context.Background(), &corev1.Namespace{
148-
ObjectMeta: metav1.ObjectMeta{Name: "venafi"},
149-
}))
150-
require.NoError(t, kclient.Create(context.Background(), &corev1.ServiceAccount{
151-
ObjectMeta: metav1.ObjectMeta{Name: "venafi-connection", Namespace: "venafi"},
152-
}))
153-
require.NoError(t, kclient.Create(context.Background(), &rbacv1.ClusterRole{
154-
ObjectMeta: metav1.ObjectMeta{Name: "venafi-connection-role"},
155-
Rules: []rbacv1.PolicyRule{
156-
{APIGroups: []string{""}, Resources: []string{"namespaces"}, Verbs: []string{"get", "list", "watch"}},
157-
{APIGroups: []string{"jetstack.io"}, Resources: []string{"venaficonnections"}, Verbs: []string{"get", "list", "watch"}},
158-
{APIGroups: []string{"jetstack.io"}, Resources: []string{"venaficonnections/status"}, Verbs: []string{"get", "patch"}},
159-
},
160-
}))
161-
require.NoError(t, kclient.Create(context.Background(), &rbacv1.ClusterRoleBinding{
162-
ObjectMeta: metav1.ObjectMeta{Name: "venafi-connection-rolebinding"},
163-
RoleRef: rbacv1.RoleRef{APIGroup: "rbac.authorization.k8s.io", Kind: "ClusterRole", Name: "venafi-connection-role"},
164-
Subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Name: "venafi-connection", Namespace: "venafi"}},
165-
}))
166-
require.NoError(t, kclient.Create(context.Background(), &corev1.Secret{
167-
ObjectMeta: metav1.ObjectMeta{Name: "accesstoken", Namespace: "venafi"},
168-
StringData: map[string]string{"accesstoken": "VALID_ACCESS_TOKEN"},
169-
}))
170-
require.NoError(t, kclient.Create(context.Background(), &corev1.Secret{
171-
ObjectMeta: metav1.ObjectMeta{Name: "apikey", Namespace: "venafi"},
172-
StringData: map[string]string{"apikey": "VALID_API_KEY"},
173-
}))
174-
require.NoError(t, kclient.Create(context.Background(), &rbacv1.Role{
175-
ObjectMeta: metav1.ObjectMeta{Name: "venafi-connection-secret-reader", Namespace: "venafi"},
176-
Rules: []rbacv1.PolicyRule{
177-
{APIGroups: []string{""}, Resources: []string{"secrets"}, Verbs: []string{"get"}, ResourceNames: []string{"accesstoken", "apikey"}},
178-
},
179-
}))
180-
require.NoError(t, kclient.Create(context.Background(), &rbacv1.RoleBinding{
181-
ObjectMeta: metav1.ObjectMeta{Name: "venafi-connection-secret-reader", Namespace: "venafi"},
182-
RoleRef: rbacv1.RoleRef{APIGroup: "rbac.authorization.k8s.io", Kind: "Role", Name: "venafi-connection-secret-reader"},
183-
Subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Name: "venafi-connection", Namespace: "venafi"}},
184-
}))
185207

186208
test.given = strings.ReplaceAll(test.given, "FAKE_VENAFI_CLOUD_URL", fakeVenafiCloud.URL)
187209
test.given = strings.ReplaceAll(test.given, "FAKE_TPP_URL", fakeTPP.URL)
188-
for _, obj := range parse(test.given) {
210+
211+
var given []ctrlruntime.Object
212+
given = append(given, parse(rbac)...)
213+
given = append(given, parse(undent(`
214+
apiVersion: v1
215+
kind: Secret
216+
metadata:
217+
name: accesstoken
218+
namespace: venafi
219+
stringData:
220+
accesstoken: VALID_ACCESS_TOKEN
221+
---
222+
apiVersion: v1
223+
kind: Secret
224+
metadata:
225+
name: apikey
226+
namespace: venafi
227+
stringData:
228+
apikey: VALID_API_KEY
229+
---
230+
apiVersion: rbac.authorization.k8s.io/v1
231+
kind: Role
232+
metadata:
233+
name: venafi-connection-secret-reader
234+
namespace: venafi
235+
rules:
236+
- apiGroups: [""]
237+
resources: ["secrets"]
238+
verbs: ["get"]
239+
resourceNames: ["accesstoken", "apikey"]
240+
---
241+
apiVersion: rbac.authorization.k8s.io/v1
242+
kind: RoleBinding
243+
metadata:
244+
name: venafi-connection-secret-reader
245+
namespace: venafi
246+
roleRef:
247+
apiGroup: rbac.authorization.k8s.io
248+
kind: Role
249+
name: venafi-connection-secret-reader
250+
subjects:
251+
- kind: ServiceAccount
252+
name: venafi-connection
253+
namespace: venafi`))...)
254+
given = append(given, parse(test.given)...)
255+
for _, obj := range given {
189256
require.NoError(t, kclient.Create(context.Background(), obj))
190257
}
191258

‎pkg/datagatherer/k8s/client.go

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,15 @@ import (
66
"k8s.io/client-go/discovery"
77
"k8s.io/client-go/dynamic"
88
"k8s.io/client-go/kubernetes"
9-
"k8s.io/client-go/rest"
10-
"k8s.io/client-go/tools/clientcmd"
9+
10+
"github.com/jetstack/preflight/pkg/kubeconfig"
1111
)
1212

1313
// NewDynamicClient creates a new 'dynamic' clientset using the provided kubeconfig.
1414
// If kubeconfigPath is not set/empty, it will attempt to load configuration using
1515
// the default loading rules.
1616
func NewDynamicClient(kubeconfigPath string) (dynamic.Interface, error) {
17-
cfg, err := loadRESTConfig(kubeconfigPath)
17+
cfg, err := kubeconfig.LoadRESTConfig(kubeconfigPath)
1818
if err != nil {
1919
return nil, errors.WithStack(err)
2020
}
@@ -31,7 +31,7 @@ func NewDynamicClient(kubeconfigPath string) (dynamic.Interface, error) {
3131
func NewDiscoveryClient(kubeconfigPath string) (discovery.DiscoveryClient, error) {
3232
var discoveryClient *discovery.DiscoveryClient
3333

34-
cfg, err := loadRESTConfig(kubeconfigPath)
34+
cfg, err := kubeconfig.LoadRESTConfig(kubeconfigPath)
3535
if err != nil {
3636
return discovery.DiscoveryClient{}, errors.WithStack(err)
3737
}
@@ -49,7 +49,7 @@ func NewDiscoveryClient(kubeconfigPath string) (discovery.DiscoveryClient, error
4949
// the default loading rules.
5050
func NewClientSet(kubeconfigPath string) (kubernetes.Interface, error) {
5151
var clientset *kubernetes.Clientset
52-
cfg, err := loadRESTConfig(kubeconfigPath)
52+
cfg, err := kubeconfig.LoadRESTConfig(kubeconfigPath)
5353
if err != nil {
5454
return nil, errors.WithStack(err)
5555
}
@@ -59,28 +59,3 @@ func NewClientSet(kubeconfigPath string) (kubernetes.Interface, error) {
5959
}
6060
return clientset, nil
6161
}
62-
63-
func loadRESTConfig(path string) (*rest.Config, error) {
64-
switch path {
65-
// If the kubeconfig path is not provided, use the default loading rules
66-
// so we read the regular KUBECONFIG variable or create a non-interactive
67-
// client for agents running in cluster
68-
case "":
69-
loadingrules := clientcmd.NewDefaultClientConfigLoadingRules()
70-
cfg, err := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(
71-
loadingrules, &clientcmd.ConfigOverrides{}).ClientConfig()
72-
if err != nil {
73-
return nil, errors.WithStack(err)
74-
}
75-
return cfg, nil
76-
// Otherwise use the explicitly named kubeconfig file.
77-
default:
78-
cfg, err := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(
79-
&clientcmd.ClientConfigLoadingRules{ExplicitPath: path},
80-
&clientcmd.ConfigOverrides{}).ClientConfig()
81-
if err != nil {
82-
return nil, errors.WithStack(err)
83-
}
84-
return cfg, nil
85-
}
86-
}

‎pkg/kubeconfig/kubeconfig.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package kubeconfig
2+
3+
import (
4+
"github.com/pkg/errors"
5+
"k8s.io/client-go/rest"
6+
"k8s.io/client-go/tools/clientcmd"
7+
)
8+
9+
// LoadRESTConfig loads the kube config from the provided path. If the path is
10+
// empty, the kube config will be loaded from KUBECONFIG, and if KUBECONFIG
11+
// isn't set, the in-cluster config will be used.
12+
func LoadRESTConfig(path string) (*rest.Config, error) {
13+
switch path {
14+
// If the kubeconfig path is not provided, use the default loading rules
15+
// so we read the regular KUBECONFIG variable or create a non-interactive
16+
// client for agents running in cluster
17+
case "":
18+
loadingrules := clientcmd.NewDefaultClientConfigLoadingRules()
19+
cfg, err := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(
20+
loadingrules, &clientcmd.ConfigOverrides{}).ClientConfig()
21+
if err != nil {
22+
return nil, errors.WithStack(err)
23+
}
24+
return cfg, nil
25+
// Otherwise use the explicitly named kubeconfig file.
26+
default:
27+
cfg, err := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(
28+
&clientcmd.ClientConfigLoadingRules{ExplicitPath: path},
29+
&clientcmd.ConfigOverrides{}).ClientConfig()
30+
if err != nil {
31+
return nil, errors.WithStack(err)
32+
}
33+
return cfg, nil
34+
}
35+
}

0 commit comments

Comments
 (0)