Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion pkg/cmd/tknpac/create/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,29 @@ func (r *RepoOptions) Create(ctx context.Context) (string, string, error) {
return repoName, repoNamespace, err
}

// checkSystemNamespace returns an error for system namespaces and prints a warning for kube-* prefixes.
func checkSystemNamespace(ns string, opts *RepoOptions) error {
if ns == "openshift-pipelines" || ns == "tekton-pipelines" {
Comment thread
krishhlogan marked this conversation as resolved.
return fmt.Errorf("namespace %s is not supported as a target for repositories", ns)
}
if strings.HasPrefix(ns, "openshift-") {
Comment thread
krishhlogan marked this conversation as resolved.
return fmt.Errorf("cannot create a repository in namespace %s as system namespaces are not supported", ns)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] The system-namespace protection is applied inconsistently: any openshift--prefixed namespace is hard-blocked, but kube--prefixed namespaces (e.g. kube-system, kube-public, kube-node-lease β€” arguably the most sensitive Kubernetes system namespaces) and non-exact tekton--prefixed namespaces only produce a warning and still allow repository creation to proceed. If the intent of this change is to prevent accidental repository creation in system namespaces, kube- should likely be hard-blocked the same way openshift- is, rather than just warned about.

if strings.HasPrefix(ns, "kube-") {
fmt.Fprintf(opts.IoStreams.Out, "%s Warning: namespace %s has prefix kube- which is a system namespace\n",
opts.IoStreams.ColorScheme().WarningIcon(), ns)
}
if strings.HasPrefix(ns, "tekton-") {
fmt.Fprintf(opts.IoStreams.Out, "%s Warning: namespace %s has prefix tekton- which is a system namespace\n",
opts.IoStreams.ColorScheme().WarningIcon(), ns)
}
return nil
}

// getOrCreateNamespace ask and create namespace or use the default one.
func getOrCreateNamespace(ctx context.Context, opts *RepoOptions) error {
if opts.Repository.Namespace != "" {
return nil
return checkSystemNamespace(opts.Repository.Namespace, opts)
}

// by default, use the current namespace unless it's default or
Expand All @@ -179,6 +198,9 @@ func getOrCreateNamespace(ctx context.Context, opts *RepoOptions) error {
if chosenNS == "" {
chosenNS = autoNS
}
if err := checkSystemNamespace(chosenNS, opts); err != nil {
return err
}
// check if the namespace exists if it does just exit
_, err := opts.Run.Clients.Kube.CoreV1().Namespaces().Get(ctx, chosenNS, metav1.GetOptions{})
if err == nil {
Expand Down
87 changes: 87 additions & 0 deletions pkg/cmd/tknpac/create/repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,93 @@ func TestGetNamespace(t *testing.T) {
Kube: &info.KubeOpts{},
},
},
{
name: "error on openshift-pipelines namespace via prompt",
askStubs: func(as *prompt.AskStubber) {
as.StubOne("openshift-pipelines")
},
wantErrStr: "namespace openshift-pipelines is not supported as a target for repositories",
runInfo: info.Info{
Kube: &info.KubeOpts{},
},
},
{
name: "error on tekton-pipelines namespace via prompt",
askStubs: func(as *prompt.AskStubber) {
as.StubOne("tekton-pipelines")
},
wantErrStr: "namespace tekton-pipelines is not supported as a target for repositories",
runInfo: info.Info{
Kube: &info.KubeOpts{},
},
},
{
name: "error on openshift system namespace prefix via prompt",
askStubs: func(as *prompt.AskStubber) {
as.StubOne("openshift-foo")
},
wantErrStr: "cannot create a repository in namespace openshift-foo as system namespaces are not supported",
runInfo: info.Info{
Kube: &info.KubeOpts{},
},
},
{
name: "warn on kube system namespace via prompt",
askStubs: func(as *prompt.AskStubber) {
as.StubOne("kube-system")
as.StubOne(true)
},
wantStdout: "! Warning: namespace kube-system has prefix kube- which is a system namespace\n! Namespace kube-system is not found",
runInfo: info.Info{
Kube: &info.KubeOpts{},
},
},
{
name: "error on openshift-pipelines namespace via flag",
Comment thread
krishhlogan marked this conversation as resolved.
repo: apipac.Repository{
ObjectMeta: metav1.ObjectMeta{Namespace: "openshift-pipelines"},
},
wantErrStr: "namespace openshift-pipelines is not supported as a target for repositories",
},
{
name: "warn on kube system namespace via flag",
repo: apipac.Repository{
ObjectMeta: metav1.ObjectMeta{Namespace: "kube-system"},
},
wantStdout: "! Warning: namespace kube-system has prefix kube- which is a system namespace",
},
{
name: "error on tekton-pipelines namespace via flag",
repo: apipac.Repository{
ObjectMeta: metav1.ObjectMeta{Namespace: "tekton-pipelines"},
},
wantErrStr: "namespace tekton-pipelines is not supported as a target for repositories",
},
{
name: "error on openshift system namespace prefix via flag",
repo: apipac.Repository{
ObjectMeta: metav1.ObjectMeta{Namespace: "openshift-foo"},
},
wantErrStr: "cannot create a repository in namespace openshift-foo as system namespaces are not supported",
},
{
name: "warn on tekton system namespace prefix via prompt",
askStubs: func(as *prompt.AskStubber) {
as.StubOne("tekton-foo")
as.StubOne(true)
},
wantStdout: "! Warning: namespace tekton-foo has prefix tekton- which is a system namespace\n! Namespace tekton-foo is not found",
runInfo: info.Info{
Kube: &info.KubeOpts{},
},
},
{
name: "warn on tekton system namespace prefix via flag",
repo: apipac.Repository{
ObjectMeta: metav1.ObjectMeta{Namespace: "tekton-foo"},
},
wantStdout: "! Warning: namespace tekton-foo has prefix tekton- which is a system namespace",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
Loading