-
Notifications
You must be signed in to change notification settings - Fork 702
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
Migrate namespaces retrieval from Kubeops to Kubeapps APIs #5239
Changes from 3 commits
d0fd648
ad13c98
df390ae
f8a486a
2d304d5
fbf252b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
{{- if .Values.rbac.create -}} | ||
apiVersion: {{ include "common.capabilities.rbac.apiVersion" . }} | ||
kind: ClusterRole | ||
metadata: | ||
name: {{ printf "kubeapps:%s:kubeappsapis-ns-discovery" .Release.Namespace | quote }} | ||
labels: {{- include "common.labels.standard" . | nindent 4 }} | ||
app.kubernetes.io/component: kubeappsapis | ||
{{- if .Values.commonLabels }} | ||
{{- include "common.tplvalues.render" ( dict "value" .Values.commonLabels "context" . ) | nindent 4 }} | ||
{{- end }} | ||
{{- if .Values.commonAnnotations }} | ||
annotations: {{- include "common.tplvalues.render" ( dict "value" .Values.commonAnnotations "context" $ ) | nindent 4 }} | ||
{{- end }} | ||
rules: | ||
- apiGroups: | ||
- "" | ||
resources: | ||
- namespaces | ||
verbs: | ||
- list | ||
--- | ||
apiVersion: {{ include "common.capabilities.rbac.apiVersion" . }} | ||
kind: ClusterRoleBinding | ||
metadata: | ||
name: {{ printf "kubeapps:%s:kubeappsapis-ns-discovery" .Release.Namespace | quote }} | ||
labels: {{- include "common.labels.standard" . | nindent 4 }} | ||
app.kubernetes.io/component: kubeappsapis | ||
{{- if .Values.commonLabels }} | ||
{{- include "common.tplvalues.render" ( dict "value" .Values.commonLabels "context" . ) | nindent 4 }} | ||
{{- end }} | ||
{{- if .Values.commonAnnotations }} | ||
annotations: {{- include "common.tplvalues.render" ( dict "value" .Values.commonAnnotations "context" $ ) | nindent 4 }} | ||
{{- end }} | ||
roleRef: | ||
apiGroup: rbac.authorization.k8s.io | ||
kind: ClusterRole | ||
name: {{ printf "kubeapps:%s:kubeappsapis-ns-discovery" .Release.Namespace | quote }} | ||
subjects: | ||
- kind: ServiceAccount | ||
name: {{ template "kubeapps.kubeappsapis.serviceAccountName" . }} | ||
namespace: {{ .Release.Namespace }} | ||
{{- end -}} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
// Copyright 2022 the Kubeapps contributors. | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
package common | ||
|
||
import ( | ||
"encoding/json" | ||
"fmt" | ||
"os" | ||
) | ||
|
||
type ResourcesPluginConfig struct { | ||
TrustedNamespaces TrustedNamespaces | ||
} | ||
|
||
type TrustedNamespaces struct { | ||
HeaderName string | ||
HeaderPattern string | ||
} | ||
|
||
func NewDefaultPluginConfig() *ResourcesPluginConfig { | ||
// If no config is provided, we default to the existing values for backwards compatibility. | ||
return &ResourcesPluginConfig{} | ||
} | ||
|
||
// ParsePluginConfig parses the input plugin configuration json file and returns the configuration options. | ||
func ParsePluginConfig(pluginConfigPath string) (*ResourcesPluginConfig, error) { | ||
|
||
// Resources plugin config defines the following struct and json config | ||
type resourcesConfig struct { | ||
Resources struct { | ||
Packages struct { | ||
V1alpha1 struct { | ||
TrustedNamespaces struct { | ||
HeaderName string `json:"headerName"` | ||
HeaderPattern string `json:"headerPattern"` | ||
} `json:"trustedNamespaces"` | ||
} `json:"v1alpha1"` | ||
} `json:"packages"` | ||
} `json:"resources"` | ||
} | ||
var config resourcesConfig | ||
|
||
// #nosec G304 | ||
pluginConfig, err := os.ReadFile(pluginConfigPath) | ||
if err != nil { | ||
return nil, fmt.Errorf("unable to open plugin config at %q: %w", pluginConfigPath, err) | ||
} | ||
err = json.Unmarshal(pluginConfig, &config) | ||
if err != nil { | ||
return nil, fmt.Errorf("unable to unmarshal pluginconfig: %q error: %w", string(pluginConfig), err) | ||
} | ||
|
||
// return configured value | ||
return &ResourcesPluginConfig{ | ||
TrustedNamespaces: TrustedNamespaces{ | ||
HeaderName: config.Resources.Packages.V1alpha1.TrustedNamespaces.HeaderName, | ||
HeaderPattern: config.Resources.Packages.V1alpha1.TrustedNamespaces.HeaderPattern, | ||
}, | ||
}, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
// Copyright 2022 the Kubeapps contributors. | ||
// SPDX-License-Identifier: Apache-2.0 | ||
package common | ||
castelblanque marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
import ( | ||
"github.com/google/go-cmp/cmp" | ||
"github.com/google/go-cmp/cmp/cmpopts" | ||
"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/pkgutils" | ||
log "k8s.io/klog/v2" | ||
"os" | ||
"runtime" | ||
"sigs.k8s.io/yaml" | ||
"strings" | ||
"testing" | ||
) | ||
|
||
func TestParsePluginConfig(t *testing.T) { | ||
testCases := []struct { | ||
name string | ||
pluginYAMLConf []byte | ||
expectedConfig *ResourcesPluginConfig | ||
expectedError string | ||
}{ | ||
{ | ||
name: "non existing plugin-config file", | ||
pluginYAMLConf: nil, | ||
expectedConfig: &ResourcesPluginConfig{}, | ||
expectedError: "", | ||
}, | ||
{ | ||
name: "invalid plugin config", | ||
pluginYAMLConf: []byte(` | ||
resources: | ||
packages: | ||
v1alpha1: | ||
trustedNamespaces: | ||
headerName: true | ||
`), | ||
expectedConfig: nil, | ||
expectedError: "json: cannot unmarshal", | ||
}, | ||
{ | ||
name: "non-default, valid plugin config", | ||
pluginYAMLConf: []byte(` | ||
resources: | ||
packages: | ||
v1alpha1: | ||
trustedNamespaces: | ||
headerName: "X-Consumer-Groups" | ||
headerPattern: "^namespace:([\\w-]+)$" | ||
`), | ||
expectedConfig: &ResourcesPluginConfig{ | ||
TrustedNamespaces: TrustedNamespaces{ | ||
HeaderName: "X-Consumer-Groups", | ||
HeaderPattern: "^namespace:([\\w-]+)$", | ||
}, | ||
}, | ||
expectedError: "", | ||
}, | ||
} | ||
opts := cmpopts.IgnoreUnexported(pkgutils.VersionsInSummary{}) | ||
for _, tc := range testCases { | ||
t.Run(tc.name, func(t *testing.T) { | ||
// TODO(agamez): env vars and file paths should be handled properly for Windows operating system | ||
if runtime.GOOS == "windows" { | ||
t.Skip("Skipping in a Windows OS") | ||
} | ||
filename := "" | ||
if tc.pluginYAMLConf != nil { | ||
pluginJSONConf, err := yaml.YAMLToJSON(tc.pluginYAMLConf) | ||
if err != nil { | ||
log.Fatalf("%s", err) | ||
} | ||
Comment on lines
+71
to
+73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Haven't you considered the use of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitely, I'll try to remember next time. In many cases, this block of code is just copy-pasted. |
||
f, err := os.CreateTemp(".", "plugin_json_conf") | ||
if err != nil { | ||
log.Fatalf("%s", err) | ||
} | ||
defer os.Remove(f.Name()) // clean up | ||
if _, err := f.Write(pluginJSONConf); err != nil { | ||
log.Fatalf("%s", err) | ||
} | ||
if err := f.Close(); err != nil { | ||
log.Fatalf("%s", err) | ||
} | ||
filename = f.Name() | ||
} | ||
pluginConfig, err := ParsePluginConfig(filename) | ||
if err != nil && !strings.Contains(err.Error(), tc.expectedError) { | ||
t.Errorf("err got %q, want to find %q", err.Error(), tc.expectedError) | ||
} else if pluginConfig != nil { | ||
if got, want := pluginConfig, tc.expectedConfig; !cmp.Equal(want, got, opts) { | ||
t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got, opts)) | ||
} | ||
} | ||
}) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @castelblanque, just for the sake of learning, I've never seen this kind of comment/annotation before, and I guess it should be processed by some kind of security linter/checker. Could you tell me what's the tool in charge of this and where is its configuration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a
gosec
annotation we use for ignoring false positives and wontfixes. See https://github.com/securego/gosec#annotating-code