Skip to content

fix(cluster connect): display service account credentials #1237

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.

Sign up for GitHub

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

Merged
rkpattnaik780 merged 2 commits into from
Oct 19, 2021
Merged

fix(cluster connect): display service account credentials #1237

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9dc0c18
fix(cluster connect): display service account credentials
rkpattnaik780 Oct 19, 2021
1932fb5
fix: remove client secret and indent grant command
rkpattnaik780 Oct 19, 2021
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
8 changes: 6 additions & 2 deletions pkg/cluster/connect.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,11 @@ func (c *KubernetesClusterAPIImpl) createServiceAccountSecretIfNeeded(namespace
return fmt.Errorf("%v: %w", cliOpts.Localizer.MustLocalize("cluster.kubernetes.serviceaccountsecret.error.createError"), err)
}

cliOpts.Logger.Info(cliOpts.Localizer.MustLocalize("cluster.kubernetes.createSASecret.log.info.createSuccess", localize.NewEntry("Name", createdSecret.Name)))
cliOpts.Logger.Info(cliOpts.Localizer.MustLocalize("cluster.kubernetes.createSASecret.log.info.createSuccess",
localize.NewEntry("Name", createdSecret.Name),
localize.NewEntry("ClientID", serviceAcct.GetClientId()),
localize.NewEntry("ClientSecret", serviceAcct.GetClientSecret()),
Copy link
Contributor

@craicoverflow craicoverflow Oct 19, 2021

Choose a reason for hiding this comment

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

I don't think we should print the client secret for any reason. The CLI user only needs the client ID for ACL operations.

Copy link
Contributor Author

@rkpattnaik780 rkpattnaik780 Oct 19, 2021

Choose a reason for hiding this comment

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

Though it may be irrelevant for cluster context, I think it will be useful if user needs a svc-account created like this for other purposes. wdyt?

Copy link
Contributor

@craicoverflow craicoverflow Oct 19, 2021

Choose a reason for hiding this comment

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

It's risky, it is printing a password that gives access to data. Default to safe, if people need it we can propose another solution. This is why the service-account commands do not print anything and you must specify a file.

Copy link
Contributor Author

@rkpattnaik780 rkpattnaik780 Oct 19, 2021

Choose a reason for hiding this comment

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

Okay. Taking it down.

))

return nil
}
Expand Down Expand Up @@ -288,7 +292,7 @@ func (api *KubernetesClusterAPIImpl) watchForServiceStatus(

case <-time.After(60 * time.Second):
w.Stop()
return fmt.Errorf(localizer.MustLocalize("cluster.kubernetes.watchForKafkaStatus.error.timeout"))
return fmt.Errorf(localizer.MustLocalize("cluster.kubernetes.watchForResourceStatus.error.timeout", localize.NewEntry("Resource", serviceDetails.Type)))
}
}
}
16 changes: 14 additions & 2 deletions pkg/localize/locales/en/cmd/cluster.en.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ one = '''
Connection Details:

Service Type: {{.ServiceType}}
Service Name: {{.ServiceName}}
Service Name: {{.ServiceName}}
Kubernetes Namespace: {{.Namespace}}
Service Account Secret: {{.ServiceAccountSecretName}}
'''
Expand Down Expand Up @@ -288,7 +288,19 @@ one = 'Access token already exist on the specified namespace'
one = 'Token Secret "{{.Name}}" created successfully'

[cluster.kubernetes.createSASecret.log.info.createSuccess]
one = 'Service Account Secret "{{.Name}}" created successfully'
one = '''
Service Account Secret "{{.Name}}" created successfully

Client ID: {{.ClientID}}
Client Secret: {{.ClientSecret}}
Copy link
Contributor

@craicoverflow craicoverflow Oct 19, 2021

Choose a reason for hiding this comment

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

I think we can omit that


Make a copy of the client ID and secret to store in a safe place. Credentials won't appear again after closing the terminal.

Execute the following command to grant access to the service-account using rhoas cli

rhoas kafka acl grant-access --producer --consumer --service-account {{.ClientID}} --topic "*" --group "*"
Copy link
Contributor

@craicoverflow craicoverflow Oct 19, 2021

Choose a reason for hiding this comment

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

If we externalize this as a variable, we could highlight it in colour to make it easier to identify and keep separate from text.

Copy link
Contributor

@craicoverflow craicoverflow Oct 19, 2021

Choose a reason for hiding this comment

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

Or simply indent it to differentiate.


'''

[cluster.kubernetes.createTokenSecret.log.info.createFailed]
one = 'Creation of the "{{.Name}}" secret failed:'
Expand Down