[WIP] Create new namespace pull secret based on namespace pull robot account #267
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
rcerven
force-pushed
the
namespacerobot
branch
2 times, most recently
from
e1e02af to
58b370f
Compare
which is granted read permisssions for all ImageRepositories in the namespace, namespace pull secret is also linked to component build SA and integration SA, new annotation 'image-controller.appstudio.redhat.com/ensure-namespace-pull-secret' is introduced as well, which is set to 'false' after namespace pull secret is created, when set to 'true' it will force to create namespace pull robot account and namespace secret, component linking check is based now only on component label, as new model won't have anymore application STONEBLD-4018 Signed-off-by: Robert Cerven <[email protected]>
rcerven
force-pushed
the
namespacerobot
branch
from
58b370f to
b4b1066
Compare
mmorhun
left a comment
mmorhun
left a comment
There was a problem hiding this comment.
Looks good in general, a few remarks below.
| } | ||
|
|
||
| // udateServiceAccountWithApplicationPullSecret updates the ServiceAccount to include | ||
| // updateServiceAccountWithApplicationPullSecret updates the ServiceAccount to include |
There was a problem hiding this comment.
The name is misleading and unclear.
What about updateIntegrationServiceAccountWithNamespacePullSecret?
But then why it happens in the application controller? Do we need to even have this controller?
| deleteImageRepository(imageRepositoryName) | ||
| }) | ||
|
|
||
| It("should do image repository provision, component doesn't have application", func() { |
There was a problem hiding this comment.
nitpick: I'd replace , with when
| "k8s.io/apimachinery/pkg/types" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
|
|
||
| appstudioapiv1alpha1 "github.com/konflux-ci/application-api/api/v1alpha1" |
| CreateRobotAccount(organization string, robotName string) (*RobotAccount, error) | ||
| DeleteRobotAccount(organization string, robotName string) (bool, error) | ||
| AddPermissionsForRepositoryToAccount(organization, imageRepository, accountName string, isRobot, isWrite bool) error | ||
| RemovePermissionsToRepositoryForAccount(organization, imageRepository, accountName string, isRobot bool) error |
There was a problem hiding this comment.
I think RemovePermissionsForRepositoryFromAccount would be consistent with other method names.
| } | ||
| } | ||
|
|
||
| func TestRemovePermissionsForRobotAccount(t *testing.T) { |
There was a problem hiding this comment.
ForRobotAccount -> FromRobotAccount
| namespaceRobotName, err := r.getNamespaceRobotName(ctx, imageRepository.Namespace) | ||
| if err == nil { |
| if imageRepository.Annotations == nil { | ||
| imageRepository.Annotations = make(map[string]string) | ||
| } | ||
| imageRepository.Annotations[ensureNamespacePullSecretAnnotation] = "false" | ||
|
|
There was a problem hiding this comment.
Do we need this? Does it make sense to assume that missing annotation is the same as false?
| log := ctrllog.FromContext(ctx).WithName("RegenerateNamespaceRobotAccessToken") | ||
| ctx = ctrllog.IntoContext(ctx, log) | ||
|
|
||
| quayImageURL := fmt.Sprintf("quay.io/%s/%s", r.QuayOrganization, imageRepository.Namespace) |
There was a problem hiding this comment.
nitpick: does it make sense to move it closer to the usage below?
| hostnameParts := strings.Split(hostname, ".") | ||
| hostnameLen := len(hostnameParts) | ||
|
|
||
| if hostnameLen < 4 { |
There was a problem hiding this comment.
An example and / or a comment would be nice to have here.
2 participants
which is granted read permisssions for all ImageRepositories in the namespace,
namespace pull secret is also linked to component build SA and integration SA,
new annotation
'image-controller.appstudio.redhat.com/ensure-namespace-pull-secret' is introduced as well, which is set to 'false' after namespace pull secret is created,
when set to 'true' it will force to create namespace pull robot account and namespace secret,
component linking check is based now only on component label, as new model won't have anymore application
STONEBLD-4018