Add sensible `ServiceAccount`s, `Role`s, and `RoleBinding`s to boilerplate project setup.
Add user / role / permission management capabilities to the CLI and UI.
|
-| Production Readiness | chore |
Prioritize stability of existing features.
Pay down technical debt.
**This is not a guarantee that v0.6.0 will be production-ready. It is a commitment to a large step in that direction.**
|
-| [Patch Promotions](https://github.com/akuity/kargo/issues/1250) | poc | Support a generalized option to promote arbitrary configuration (e.g. strings, files, and directories) to other paths of the Git repository. |
+| [Patch Promotions](https://github.com/akuity/kargo/issues/1250) | poc | Support a generalized option to promote arbitrary configuration (e.g. strings, files, and directories) to other paths of a GitOps repository. |
+| Production Readiness | chore |
Prioritize stability of existing features.
Pay down technical debt.
**This is not a guarantee that v0.7.0 will be production-ready. It is a commitment to large steps in that direction.**
|
-## v0.7.0 .. v0.n.0
+## v0.8.0 .. v0.n.0
| Name | Type | Description |
| ---- | ---- | ----------- |
@@ -74,7 +88,7 @@ __Status:__ In Progress
| `kargo init` | feature | Addition of an `init` sub-command to the Kargo CLI for streamlining project / pipeline creation. |
| Standalone Image Writeback | feature | Write back image changes without having to subscribe to an image repository. |
-## Criteria for 1.0.0 Release
+## Criteria for a Production-Ready 1.0.0 Release
Maintainers will consider cutting a stable v1.0.0 release once:
diff --git a/internal/api/rbac/policy_rules.go b/internal/api/rbac/policy_rules.go
index 69543185b..5422b3a68 100644
--- a/internal/api/rbac/policy_rules.go
+++ b/internal/api/rbac/policy_rules.go
@@ -7,6 +7,10 @@ import (
"strings"
rbacv1 "k8s.io/api/rbac/v1"
+ kubeerr "k8s.io/apimachinery/pkg/api/errors"
+
+ kargoapi "github.com/akuity/kargo/api/v1alpha1"
+ rolloutsapi "github.com/akuity/kargo/internal/controller/rollouts/api/v1alpha1"
)
var allVerbs = []string{
@@ -49,27 +53,23 @@ func BuildNormalizedPolicyRulesMap(
) (map[string]rbacv1.PolicyRule, error) {
rulesMap := make(map[string]rbacv1.PolicyRule)
for _, rule := range rules {
- for _, group := range rule.APIGroups {
- group = strings.TrimSpace(group)
- if group == "*" {
- return nil, fmt.Errorf("wildcard APIGroup is not allowed")
+ for _, resource := range rule.Resources {
+ if err := validateResourceTypeName(resource); err != nil {
+ return nil, err
}
- for _, resource := range rule.Resources {
- resource = strings.TrimSpace(resource)
- if resource == "*" {
- return nil, fmt.Errorf("wildcard Resource is not allowed")
- }
- if len(rule.ResourceNames) == 0 {
- rule.ResourceNames = append(rule.ResourceNames, "")
- }
- for _, resourceName := range rule.ResourceNames {
- verbs := rule.Verbs
- key := RuleKey(group, resource, resourceName)
- if existingRule, ok := rulesMap[key]; ok {
- verbs = append(existingRule.Verbs, verbs...)
- }
- rulesMap[key] = buildRule(group, resource, resourceName, verbs)
+ // We ignore the group in the rule and use what we know to be the correct
+ // group for the resource type.
+ group := getGroupName(resource)
+ if len(rule.ResourceNames) == 0 {
+ rule.ResourceNames = append(rule.ResourceNames, "")
+ }
+ for _, resourceName := range rule.ResourceNames {
+ verbs := rule.Verbs
+ key := RuleKey(group, resource, resourceName)
+ if existingRule, ok := rulesMap[key]; ok {
+ verbs = append(existingRule.Verbs, verbs...)
}
+ rulesMap[key] = buildRule(group, resource, resourceName, verbs)
}
}
}
@@ -150,3 +150,40 @@ func buildRule(
}
return rule
}
+
+// nolint: goconst
+func validateResourceTypeName(resource string) error {
+ switch resource {
+ case "analysisruns", "analysistemplates", "events", "freights", "freights/status", "roles",
+ "rolebindings", "promotions", "secrets", "serviceaccounts", "stages", "warehouses":
+ return nil
+ case "analysisrun", "analysistemplate", "event", "freight", "role",
+ "rolebinding", "promotion", "secret", "serviceaccount", "stage", "warehouse":
+ return kubeerr.NewBadRequest(
+ fmt.Sprintf(`unrecognized resource type %q; did you mean "%ss"?`, resource, resource),
+ )
+ case "freight/status":
+ return kubeerr.NewBadRequest(
+ `unrecognized resource type "freight/status"; did you mean "freights/status"?`,
+ )
+ default:
+ return kubeerr.NewBadRequest(fmt.Sprintf(`unrecognized resource type %q`, resource))
+ }
+}
+
+// nolint: goconst
+func getGroupName(resourceType string) string {
+ // resourceType must already be validated
+ switch resourceType {
+ case "events", "secrets", "serviceaccounts":
+ return ""
+ case "rolebindings", "roles":
+ return rbacv1.SchemeGroupVersion.Group
+ case "freights", "freights/status", "promotions", "stages", "warehouses":
+ return kargoapi.GroupVersion.Group
+ case "analysisruns", "analysistemplates":
+ return rolloutsapi.GroupVersion.Group
+ default:
+ return "" // If the resourceType was validated, this will never happen
+ }
+}
diff --git a/internal/api/rbac/policy_rules_test.go b/internal/api/rbac/policy_rules_test.go
index b8249d016..5dd2f7a6d 100644
--- a/internal/api/rbac/policy_rules_test.go
+++ b/internal/api/rbac/policy_rules_test.go
@@ -10,61 +10,34 @@ import (
)
func TestNormalizePolicyRules(t *testing.T) {
-
- t.Run("wildcard group not allowed", func(t *testing.T) {
+ t.Run("invalid resource type", func(t *testing.T) {
_, err := NormalizePolicyRules([]rbacv1.PolicyRule{
{
- APIGroups: []string{"*"},
- Resources: []string{"pods"},
+ APIGroups: []string{""},
+ Resources: []string{"fake-resource"},
Verbs: []string{"get"},
},
})
- require.ErrorContains(t, err, "wildcard APIGroup is not allowed")
+ require.ErrorContains(t, err, "unrecognized resource type")
})
- t.Run("wildcard resource not allowed", func(t *testing.T) {
+ t.Run("singular resource type", func(t *testing.T) {
_, err := NormalizePolicyRules([]rbacv1.PolicyRule{
{
APIGroups: []string{""},
- Resources: []string{"*"},
- Verbs: []string{"get"},
- },
- })
- require.ErrorContains(t, err, "wildcard Resource is not allowed")
- })
-
- t.Run("multiple groups expand", func(t *testing.T) {
- rules, err := NormalizePolicyRules([]rbacv1.PolicyRule{
- { // Never mind that this doesn't make sense
- APIGroups: []string{"", rbacv1.GroupName},
- Resources: []string{"pods"},
+ Resources: []string{"stage"},
Verbs: []string{"get"},
},
})
- require.NoError(t, err)
- require.Equal(
- t,
- []rbacv1.PolicyRule{
- {
- APIGroups: []string{""},
- Resources: []string{"pods"},
- Verbs: []string{"get"},
- },
- {
- APIGroups: []string{rbacv1.GroupName},
- Resources: []string{"pods"},
- Verbs: []string{"get"},
- },
- },
- rules,
- )
+ require.ErrorContains(t, err, `unrecognized resource type "stage"`)
+ require.ErrorContains(t, err, `did you mean "stages"`)
})
t.Run("multiple resources expand", func(t *testing.T) {
rules, err := NormalizePolicyRules([]rbacv1.PolicyRule{
{
APIGroups: []string{""},
- Resources: []string{"pods", "services"},
+ Resources: []string{"secrets", "serviceaccounts"},
Verbs: []string{"get"},
},
})
@@ -74,12 +47,12 @@ func TestNormalizePolicyRules(t *testing.T) {
[]rbacv1.PolicyRule{
{
APIGroups: []string{""},
- Resources: []string{"pods"},
+ Resources: []string{"secrets"},
Verbs: []string{"get"},
},
{
APIGroups: []string{""},
- Resources: []string{"services"},
+ Resources: []string{"serviceaccounts"},
Verbs: []string{"get"},
},
},
@@ -91,7 +64,7 @@ func TestNormalizePolicyRules(t *testing.T) {
rules, err := NormalizePolicyRules([]rbacv1.PolicyRule{
{
APIGroups: []string{""},
- Resources: []string{"pods"},
+ Resources: []string{"serviceaccounts"},
ResourceNames: []string{"foo", "bar"},
Verbs: []string{"get"},
},
@@ -102,13 +75,13 @@ func TestNormalizePolicyRules(t *testing.T) {
[]rbacv1.PolicyRule{
{
APIGroups: []string{""},
- Resources: []string{"pods"},
+ Resources: []string{"serviceaccounts"},
ResourceNames: []string{"bar"},
Verbs: []string{"get"},
},
{
APIGroups: []string{""},
- Resources: []string{"pods"},
+ Resources: []string{"serviceaccounts"},
ResourceNames: []string{"foo"},
Verbs: []string{"get"},
},
@@ -121,7 +94,7 @@ func TestNormalizePolicyRules(t *testing.T) {
rules, err := NormalizePolicyRules([]rbacv1.PolicyRule{
{
APIGroups: []string{""},
- Resources: []string{"pods"},
+ Resources: []string{"serviceaccounts"},
Verbs: []string{"list", "get"},
},
})
@@ -131,7 +104,7 @@ func TestNormalizePolicyRules(t *testing.T) {
[]rbacv1.PolicyRule{
{
APIGroups: []string{""},
- Resources: []string{"pods"},
+ Resources: []string{"serviceaccounts"},
Verbs: []string{"get", "list"},
},
},
@@ -143,7 +116,7 @@ func TestNormalizePolicyRules(t *testing.T) {
rules, err := NormalizePolicyRules([]rbacv1.PolicyRule{
{
APIGroups: []string{""},
- Resources: []string{"pods"},
+ Resources: []string{"serviceaccounts"},
Verbs: []string{"get", "get"},
},
})
@@ -153,7 +126,7 @@ func TestNormalizePolicyRules(t *testing.T) {
[]rbacv1.PolicyRule{
{
APIGroups: []string{""},
- Resources: []string{"pods"},
+ Resources: []string{"serviceaccounts"},
Verbs: []string{"get"},
},
},
@@ -165,7 +138,7 @@ func TestNormalizePolicyRules(t *testing.T) {
rules, err := NormalizePolicyRules([]rbacv1.PolicyRule{
{
APIGroups: []string{""},
- Resources: []string{"pods"},
+ Resources: []string{"serviceaccounts"},
Verbs: []string{"*"},
},
})
@@ -175,7 +148,7 @@ func TestNormalizePolicyRules(t *testing.T) {
[]rbacv1.PolicyRule{
{
APIGroups: []string{""},
- Resources: []string{"pods"},
+ Resources: []string{"serviceaccounts"},
Verbs: allVerbs,
},
},
@@ -183,21 +156,43 @@ func TestNormalizePolicyRules(t *testing.T) {
)
})
+ t.Run("correct groups are determined automatically", func(t *testing.T) {
+ rules, err := NormalizePolicyRules([]rbacv1.PolicyRule{
+ {
+ APIGroups: []string{"", "foo", "bar"},
+ Resources: []string{"stages"},
+ Verbs: []string{"get"},
+ },
+ })
+ require.NoError(t, err)
+ require.Equal(
+ t,
+ []rbacv1.PolicyRule{
+ {
+ APIGroups: []string{kargoapi.GroupVersion.Group},
+ Resources: []string{"stages"},
+ Verbs: []string{"get"},
+ },
+ },
+ rules,
+ )
+ })
+
t.Run("kitchen sink", func(t *testing.T) {
rules, err := NormalizePolicyRules([]rbacv1.PolicyRule{
- { // Never mind that this doesn't make sense
- APIGroups: []string{"", rbacv1.GroupName},
- Resources: []string{"pods", "services"},
+ { // Never mind that this doesn't make sense. It should all get fixed
+ APIGroups: []string{""},
+ Resources: []string{"serviceaccounts", "stages"},
Verbs: []string{"*"},
},
{ // These should get de-duped
- APIGroups: []string{""},
- Resources: []string{"pods"},
+ APIGroups: []string{kargoapi.GroupVersion.Group},
+ Resources: []string{"stages"},
Verbs: []string{"*"},
},
{
APIGroups: []string{kargoapi.GroupVersion.Group},
- Resources: []string{"stages"},
+ Resources: []string{"warehouses"},
ResourceNames: []string{"foo", "bar"},
Verbs: []string{"get", "list"},
},
@@ -208,36 +203,26 @@ func TestNormalizePolicyRules(t *testing.T) {
[]rbacv1.PolicyRule{
{
APIGroups: []string{""},
- Resources: []string{"pods"},
+ Resources: []string{"serviceaccounts"},
Verbs: allVerbs,
},
{
- APIGroups: []string{""},
- Resources: []string{"services"},
+ APIGroups: []string{kargoapi.GroupVersion.Group},
+ Resources: []string{"stages"},
Verbs: allVerbs,
},
{
APIGroups: []string{kargoapi.GroupVersion.Group},
- Resources: []string{"stages"},
+ Resources: []string{"warehouses"},
ResourceNames: []string{"bar"},
Verbs: []string{"get", "list"},
},
{
APIGroups: []string{kargoapi.GroupVersion.Group},
- Resources: []string{"stages"},
+ Resources: []string{"warehouses"},
ResourceNames: []string{"foo"},
Verbs: []string{"get", "list"},
},
- {
- APIGroups: []string{rbacv1.GroupName},
- Resources: []string{"pods"},
- Verbs: allVerbs,
- },
- {
- APIGroups: []string{rbacv1.GroupName},
- Resources: []string{"services"},
- Verbs: allVerbs,
- },
},
rules,
)
diff --git a/internal/api/rbac/roles.go b/internal/api/rbac/roles.go
index 127d2a81f..83003cf7c 100644
--- a/internal/api/rbac/roles.go
+++ b/internal/api/rbac/roles.go
@@ -339,12 +339,18 @@ func (r *rolesDatabase) GrantPermissionsToRole(
return nil, err
}
+ if err = validateResourceTypeName(resourceDetails.ResourceType); err != nil {
+ return nil, err
+ }
+
+ group := getGroupName(resourceDetails.ResourceType)
+
newRole := role
if newRole == nil {
newRole = buildNewRole(project, name)
}
newRule := rbacv1.PolicyRule{
- APIGroups: []string{resourceDetails.ResourceGroup},
+ APIGroups: []string{group},
Resources: []string{resourceDetails.ResourceType},
Verbs: resourceDetails.Verbs,
}
@@ -496,10 +502,15 @@ func (r *rolesDatabase) RevokePermissionsFromRole(
slices.Sort(resourceDetails.Verbs)
resourceDetails.Verbs = slices.Compact(resourceDetails.Verbs)
+ if err = validateResourceTypeName(resourceDetails.ResourceType); err != nil {
+ return nil, err
+ }
+
+ group := getGroupName(resourceDetails.ResourceType)
+
filteredRules := make([]rbacv1.PolicyRule, 0, len(role.Rules))
for _, rule := range role.Rules {
- if rule.APIGroups[0] != resourceDetails.ResourceGroup ||
- rule.Resources[0] != resourceDetails.ResourceType ||
+ if rule.APIGroups[0] != group || rule.Resources[0] != resourceDetails.ResourceType ||
(resourceDetails.ResourceName != "" && rule.ResourceNames[0] != resourceDetails.ResourceName) {
filteredRules = append(filteredRules, rule)
continue
@@ -565,9 +576,9 @@ func (r *rolesDatabase) Update(
return nil, err
}
- amendClaimAnnotation(sa, rbacapi.AnnotationKeyOIDCSubjects, kargoRole.Subs)
- amendClaimAnnotation(sa, rbacapi.AnnotationKeyOIDCEmails, kargoRole.Emails)
- amendClaimAnnotation(sa, rbacapi.AnnotationKeyOIDCGroups, kargoRole.Groups)
+ replaceClaimAnnotation(sa, rbacapi.AnnotationKeyOIDCSubjects, kargoRole.Subs)
+ replaceClaimAnnotation(sa, rbacapi.AnnotationKeyOIDCEmails, kargoRole.Emails)
+ replaceClaimAnnotation(sa, rbacapi.AnnotationKeyOIDCGroups, kargoRole.Groups)
if err = r.client.Update(ctx, sa); err != nil {
return nil, fmt.Errorf(
"error updating ServiceAccount %q in namespace %q: %w", kargoRole.Name, kargoRole.Namespace, err,
@@ -620,6 +631,13 @@ func ResourcesToRole(
CreationTimestamp: sa.CreationTimestamp,
},
}
+
+ if isKargoManaged(sa) &&
+ (len(roles) == 0 || (len(roles) == 1 && isKargoManaged(&roles[0]))) &&
+ (len(rbs) == 0 || (len(rbs) == 1 && isKargoManaged(&rbs[0]))) {
+ kargoRole.KargoManaged = true
+ }
+
if sa.Annotations[rbacapi.AnnotationKeyOIDCSubjects] != "" {
kargoRole.Subs = strings.Split(sa.Annotations[rbacapi.AnnotationKeyOIDCSubjects], ",")
slices.Sort(kargoRole.Subs)
@@ -633,20 +651,20 @@ func ResourcesToRole(
slices.Sort(kargoRole.Groups)
}
- rules := []rbacv1.PolicyRule{}
+ kargoRole.Rules = []rbacv1.PolicyRule{}
for _, role := range roles {
- rules = append(rules, role.Rules...)
+ kargoRole.Rules = append(kargoRole.Rules, role.Rules...)
}
- var err error
- if kargoRole.Rules, err = NormalizePolicyRules(rules); err != nil {
- return nil, fmt.Errorf("error normalizing RBAC policy rules: %w", err)
- }
-
- if isKargoManaged(sa) &&
- (len(roles) == 0 || (len(roles) == 1 && isKargoManaged(&roles[0]))) &&
- (len(rbs) == 0 || (len(rbs) == 1 && isKargoManaged(&rbs[0]))) {
- kargoRole.KargoManaged = true
+ // Since we cannot make any assumptions that they only contain resource types
+ // we recognize, or that they don't do something really unusual like using a
+ // wildcard resource type, never attempt to normalize rules if any of the
+ // underlying resources are not Kargo-managed.
+ if kargoRole.KargoManaged {
+ var err error
+ if kargoRole.Rules, err = NormalizePolicyRules(kargoRole.Rules); err != nil {
+ return nil, fmt.Errorf("error normalizing RBAC policy rules: %w", err)
+ }
}
return kargoRole, nil
@@ -673,6 +691,15 @@ func RoleToResources(
return sa, role, rb, nil
}
+func replaceClaimAnnotation(sa *corev1.ServiceAccount, key string, values []string) {
+ slices.Sort(values)
+ values = slices.Compact(values)
+ if sa.Annotations == nil {
+ sa.Annotations = map[string]string{}
+ }
+ sa.Annotations[key] = strings.Join(values, ",")
+}
+
func amendClaimAnnotation(sa *corev1.ServiceAccount, key string, values []string) {
existing := sa.Annotations[key]
if existing != "" {
diff --git a/internal/api/rbac/roles_test.go b/internal/api/rbac/roles_test.go
index f73d6f1b0..e38002664 100644
--- a/internal/api/rbac/roles_test.go
+++ b/internal/api/rbac/roles_test.go
@@ -219,11 +219,20 @@ func TestGet(t *testing.T) {
rbacapi.AnnotationKeyOIDCEmails: "foo-email,bar-email",
rbacapi.AnnotationKeyOIDCGroups: "foo-group,bar-group",
}),
- plainRole([]rbacv1.PolicyRule{{
- APIGroups: []string{kargoapi.GroupVersion.Group},
- Resources: []string{"stages", "promotions"},
- Verbs: []string{"list", "get"},
- }}),
+ plainRole([]rbacv1.PolicyRule{
+ { // This rule has groups and types that we don't recognize. Let's
+ // make sure we don't choke on them. This could happen with roles
+ // that aren't Kargo-managed.
+ APIGroups: []string{"fake-group-1", "fake-group-2"},
+ Resources: []string{"fake-type-1", "fake-type-2"},
+ Verbs: []string{"get", "list"},
+ },
+ {
+ APIGroups: []string{kargoapi.GroupVersion.Group},
+ Resources: []string{"stages", "promotions"},
+ Verbs: []string{"list", "get"},
+ },
+ }),
plainRoleBinding(),
).Build()
db := NewKubernetesRolesDatabase(c)
@@ -244,16 +253,17 @@ func TestGet(t *testing.T) {
Subs: []string{"bar-sub", "foo-sub"},
Emails: []string{"bar-email", "foo-email"},
Groups: []string{"bar-group", "foo-group"},
+ // There should have been no attempt to normalize these rules
Rules: []rbacv1.PolicyRule{
{
- APIGroups: []string{kargoapi.GroupVersion.Group},
- Resources: []string{"promotions"},
+ APIGroups: []string{"fake-group-1", "fake-group-2"},
+ Resources: []string{"fake-type-1", "fake-type-2"},
Verbs: []string{"get", "list"},
},
{
APIGroups: []string{kargoapi.GroupVersion.Group},
- Resources: []string{"stages"},
- Verbs: []string{"get", "list"},
+ Resources: []string{"stages", "promotions"},
+ Verbs: []string{"list", "get"},
},
},
},
@@ -365,9 +375,8 @@ func TestGrantPermissionToRole(t *testing.T) {
testProject,
testKargoRoleName,
&rbacapi.ResourceDetails{
- ResourceGroup: "fake-group",
- ResourceType: "fake-resource-type",
- Verbs: []string{"get", "list"},
+ ResourceType: "fake-resource-type",
+ Verbs: []string{"get", "list"},
},
)
require.True(t, kubeerr.IsNotFound(err))
@@ -383,9 +392,8 @@ func TestGrantPermissionToRole(t *testing.T) {
testProject,
testKargoRoleName,
&rbacapi.ResourceDetails{
- ResourceGroup: "fake-group",
- ResourceType: "fake-resource-type",
- Verbs: []string{"get", "list"},
+ ResourceType: "fake-resource-type",
+ Verbs: []string{"get", "list"},
},
)
require.True(t, kubeerr.IsBadRequest(err))
@@ -401,9 +409,8 @@ func TestGrantPermissionToRole(t *testing.T) {
testProject,
testKargoRoleName,
&rbacapi.ResourceDetails{
- ResourceGroup: kargoapi.GroupVersion.Group,
- ResourceType: "stages",
- Verbs: []string{"get", "list"},
+ ResourceType: "stages",
+ Verbs: []string{"get", "list"},
},
)
require.NoError(t, err)
@@ -461,9 +468,8 @@ func TestGrantPermissionToRole(t *testing.T) {
testProject,
testKargoRoleName,
&rbacapi.ResourceDetails{
- ResourceGroup: kargoapi.GroupVersion.Group,
- ResourceType: "stages",
- Verbs: []string{"get", "list"},
+ ResourceType: "stages",
+ Verbs: []string{"get", "list"},
},
)
require.NoError(t, err)
@@ -563,56 +569,119 @@ func TestGrantRoleToUsers(t *testing.T) {
}
func TestList(t *testing.T) {
- c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(
- managedServiceAccount(map[string]string{
- rbacapi.AnnotationKeyOIDCSubjects: "foo-sub,bar-sub",
- rbacapi.AnnotationKeyOIDCEmails: "foo-email,bar-email",
- rbacapi.AnnotationKeyOIDCGroups: "foo-group,bar-group",
- }),
- managedRole([]rbacv1.PolicyRule{
- {
- APIGroups: []string{kargoapi.GroupVersion.Group},
- Resources: []string{"stages", "promotions"},
- Verbs: []string{"list", "get"},
- },
- }),
- managedRoleBinding(),
- ).Build()
- db := NewKubernetesRolesDatabase(c)
- kargoRoles, err := db.List(context.Background(), testProject)
- require.NoError(t, err)
- // Do not factor creation timestamp into the comparison
- now := metav1.NewTime(time.Now())
- for _, kargoRole := range kargoRoles {
- kargoRole.CreationTimestamp = now
- }
- require.Equal(
- t,
- []*rbacapi.Role{{
- ObjectMeta: metav1.ObjectMeta{
- Namespace: testProject,
- Name: testKargoRoleName,
- CreationTimestamp: now,
- },
- KargoManaged: true,
- Subs: []string{"bar-sub", "foo-sub"},
- Emails: []string{"bar-email", "foo-email"},
- Groups: []string{"bar-group", "foo-group"},
- Rules: []rbacv1.PolicyRule{
+ t.Run("with only kargo-managed roles", func(t *testing.T) {
+ c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(
+ managedServiceAccount(map[string]string{
+ rbacapi.AnnotationKeyOIDCSubjects: "foo-sub,bar-sub",
+ rbacapi.AnnotationKeyOIDCEmails: "foo-email,bar-email",
+ rbacapi.AnnotationKeyOIDCGroups: "foo-group,bar-group",
+ }),
+ managedRole([]rbacv1.PolicyRule{
{
APIGroups: []string{kargoapi.GroupVersion.Group},
- Resources: []string{"promotions"},
+ Resources: []string{"stages", "promotions"},
+ Verbs: []string{"list", "get"},
+ },
+ }),
+ managedRoleBinding(),
+ ).Build()
+ db := NewKubernetesRolesDatabase(c)
+ kargoRoles, err := db.List(context.Background(), testProject)
+ require.NoError(t, err)
+ // Do not factor creation timestamp into the comparison
+ now := metav1.NewTime(time.Now())
+ for _, kargoRole := range kargoRoles {
+ kargoRole.CreationTimestamp = now
+ }
+ require.Equal(
+ t,
+ []*rbacapi.Role{{
+ ObjectMeta: metav1.ObjectMeta{
+ Namespace: testProject,
+ Name: testKargoRoleName,
+ CreationTimestamp: now,
+ },
+ KargoManaged: true,
+ Subs: []string{"bar-sub", "foo-sub"},
+ Emails: []string{"bar-email", "foo-email"},
+ Groups: []string{"bar-group", "foo-group"},
+ Rules: []rbacv1.PolicyRule{
+ {
+ APIGroups: []string{kargoapi.GroupVersion.Group},
+ Resources: []string{"promotions"},
+ Verbs: []string{"get", "list"},
+ },
+ {
+ APIGroups: []string{kargoapi.GroupVersion.Group},
+ Resources: []string{"stages"},
+ Verbs: []string{"get", "list"},
+ },
+ },
+ }},
+ kargoRoles,
+ )
+ })
+
+ t.Run("with a non-kargo-managed role", func(t *testing.T) {
+ c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(
+ plainServiceAccount(map[string]string{
+ rbacapi.AnnotationKeyOIDCSubjects: "foo-sub,bar-sub",
+ rbacapi.AnnotationKeyOIDCEmails: "foo-email,bar-email",
+ rbacapi.AnnotationKeyOIDCGroups: "foo-group,bar-group",
+ }),
+ plainRole([]rbacv1.PolicyRule{
+ { // This rule has groups and types that we don't recognize. Let's
+ // make sure we don't choke on them. This could happen with roles
+ // that aren't Kargo-managed.
+ APIGroups: []string{"fake-group-1", "fake-group-2"},
+ Resources: []string{"fake-type-1", "fake-type-2"},
Verbs: []string{"get", "list"},
},
{
APIGroups: []string{kargoapi.GroupVersion.Group},
- Resources: []string{"stages"},
- Verbs: []string{"get", "list"},
+ Resources: []string{"stages", "promotions"},
+ Verbs: []string{"list", "get"},
},
- },
- }},
- kargoRoles,
- )
+ }),
+ plainRoleBinding(),
+ ).Build()
+ db := NewKubernetesRolesDatabase(c)
+ kargoRoles, err := db.List(context.Background(), testProject)
+ require.NoError(t, err)
+ // Do not factor creation timestamp into the comparison
+ now := metav1.NewTime(time.Now())
+ for _, kargoRole := range kargoRoles {
+ kargoRole.CreationTimestamp = now
+ }
+ require.Equal(
+ t,
+ []*rbacapi.Role{{
+ ObjectMeta: metav1.ObjectMeta{
+ Namespace: testProject,
+ Name: testKargoRoleName,
+ CreationTimestamp: now,
+ },
+ KargoManaged: false,
+ Subs: []string{"bar-sub", "foo-sub"},
+ Emails: []string{"bar-email", "foo-email"},
+ Groups: []string{"bar-group", "foo-group"},
+ // There should have been no attempt to normalize these rules
+ Rules: []rbacv1.PolicyRule{
+ {
+ APIGroups: []string{"fake-group-1", "fake-group-2"},
+ Resources: []string{"fake-type-1", "fake-type-2"},
+ Verbs: []string{"get", "list"},
+ },
+ {
+ APIGroups: []string{kargoapi.GroupVersion.Group},
+ Resources: []string{"stages", "promotions"},
+ Verbs: []string{"list", "get"},
+ },
+ },
+ }},
+ kargoRoles,
+ )
+ })
}
func TestRevokePermissionsFromRole(t *testing.T) {
@@ -624,9 +693,8 @@ func TestRevokePermissionsFromRole(t *testing.T) {
testProject,
testKargoRoleName,
&rbacapi.ResourceDetails{
- ResourceGroup: "fake-group",
- ResourceType: "fake-resource-type",
- Verbs: []string{"get", "list"},
+ ResourceType: "fake-resource-type",
+ Verbs: []string{"get", "list"},
},
)
require.True(t, kubeerr.IsNotFound(err))
@@ -642,9 +710,8 @@ func TestRevokePermissionsFromRole(t *testing.T) {
testProject,
testKargoRoleName,
&rbacapi.ResourceDetails{
- ResourceGroup: "fake-group",
- ResourceType: "fake-resource-type",
- Verbs: []string{"get", "list"},
+ ResourceType: "fake-resource-type",
+ Verbs: []string{"get", "list"},
},
)
require.True(t, kubeerr.IsBadRequest(err))
@@ -660,9 +727,8 @@ func TestRevokePermissionsFromRole(t *testing.T) {
testProject,
testKargoRoleName,
&rbacapi.ResourceDetails{
- ResourceGroup: "fake-group",
- ResourceType: "fake-resource-type",
- Verbs: []string{"get", "list"},
+ ResourceType: "fake-resource-type",
+ Verbs: []string{"get", "list"},
},
)
require.NoError(t, err)
@@ -685,9 +751,8 @@ func TestRevokePermissionsFromRole(t *testing.T) {
testProject,
testKargoRoleName,
&rbacapi.ResourceDetails{
- ResourceGroup: kargoapi.GroupVersion.Group,
- ResourceType: "stages",
- Verbs: []string{"get", "list"},
+ ResourceType: "stages",
+ Verbs: []string{"get", "list"},
},
)
require.NoError(t, err)
@@ -893,7 +958,11 @@ func TestUpdate(t *testing.T) {
t.Run("success with updated ServiceAccount and Role", func(t *testing.T) {
c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(
- managedServiceAccount(nil),
+ managedServiceAccount(map[string]string{
+ rbacapi.AnnotationKeyOIDCSubjects: "foo-sub,bar-sub",
+ rbacapi.AnnotationKeyOIDCEmails: "foo-email,bar-email",
+ rbacapi.AnnotationKeyOIDCGroups: "foo-group,bar-group",
+ }),
managedRole([]rbacv1.PolicyRule{{
APIGroups: []string{kargoapi.GroupVersion.Group},
Resources: []string{"promotions"},
@@ -909,9 +978,9 @@ func TestUpdate(t *testing.T) {
Namespace: testProject,
Name: testKargoRoleName,
},
- Subs: []string{"foo-sub", "bar-sub"},
- Emails: []string{"foo-email", "bar-email"},
- Groups: []string{"foo-group", "bar-group"},
+ Subs: []string{"foo-sub"},
+ Emails: []string{"foo-email"},
+ Groups: []string{"foo-group"},
Rules: []rbacv1.PolicyRule{{
APIGroups: []string{kargoapi.GroupVersion.Group},
Resources: []string{"stages", "promotions"},
@@ -928,9 +997,9 @@ func TestUpdate(t *testing.T) {
t,
map[string]string{
rbacapi.AnnotationKeyManaged: rbacapi.AnnotationValueTrue,
- rbacapi.AnnotationKeyOIDCSubjects: "bar-sub,foo-sub",
- rbacapi.AnnotationKeyOIDCEmails: "bar-email,foo-email",
- rbacapi.AnnotationKeyOIDCGroups: "bar-group,foo-group",
+ rbacapi.AnnotationKeyOIDCSubjects: "foo-sub",
+ rbacapi.AnnotationKeyOIDCEmails: "foo-email",
+ rbacapi.AnnotationKeyOIDCGroups: "foo-group",
},
sa.Annotations,
)
diff --git a/internal/cli/cmd/grant/grant.go b/internal/cli/cmd/grant/grant.go
index 162431a85..6e799801c 100644
--- a/internal/cli/cmd/grant/grant.go
+++ b/internal/cli/cmd/grant/grant.go
@@ -27,15 +27,14 @@ type grantOptions struct {
Config config.CLIConfig
ClientOptions client.Options
- Project string
- Role string
- Subs []string
- Emails []string
- Groups []string
- ResourceGroup string
- ResourceType string
- ResourceName string
- Verbs []string
+ Project string
+ Role string
+ Subs []string
+ Emails []string
+ Groups []string
+ ResourceType string
+ ResourceName string
+ Verbs []string
}
func NewCommand(cfg config.CLIConfig, streams genericiooptions.IOStreams) *cobra.Command {
@@ -100,7 +99,6 @@ func (o *grantOptions) addFlags(cmd *cobra.Command) {
option.Emails(cmd.Flags(), &o.Emails, "The email address of a user to be granted the role.")
option.Groups(cmd.Flags(), &o.Groups, "A group to be granted the role.")
- option.ResourceGroup(cmd.Flags(), &o.ResourceGroup, "The group of resources to grant permissions to.")
option.ResourceType(cmd.Flags(), &o.ResourceType, "A type of resource to grant permissions to.")
option.ResourceName(cmd.Flags(), &o.ResourceName, "The name of a resource to grant permissions to.")
option.Verbs(cmd.Flags(), &o.Verbs, "A verb to grant on the resource.")
@@ -114,20 +112,16 @@ func (o *grantOptions) addFlags(cmd *cobra.Command) {
option.SubFlag,
option.EmailFlag,
option.GroupFlag,
- option.ResourceGroupFlag,
+ option.ResourceTypeFlag,
)
// You can't grant a role to users and grant permissions to a role at the same
// time.
- cmd.MarkFlagsMutuallyExclusive(option.SubFlag, option.ResourceGroupFlag)
- cmd.MarkFlagsMutuallyExclusive(option.EmailFlag, option.ResourceGroupFlag)
- cmd.MarkFlagsMutuallyExclusive(option.GroupFlag, option.ResourceGroupFlag)
+ cmd.MarkFlagsMutuallyExclusive(option.SubFlag, option.ResourceTypeFlag)
+ cmd.MarkFlagsMutuallyExclusive(option.EmailFlag, option.ResourceTypeFlag)
+ cmd.MarkFlagsMutuallyExclusive(option.GroupFlag, option.ResourceTypeFlag)
- cmd.MarkFlagsRequiredTogether(
- option.ResourceGroupFlag,
- option.ResourceTypeFlag,
- option.VerbFlag,
- )
+ cmd.MarkFlagsRequiredTogether(option.ResourceTypeFlag, option.VerbFlag)
}
// validate performs validation of the options. If the options are invalid, an
@@ -156,15 +150,12 @@ func (o *grantOptions) run(ctx context.Context) error {
Project: o.Project,
Role: o.Role,
}
- // Note: Don't test if ResourceGroup is empty, because "" is a legitimate
- // value.
if o.ResourceType != "" {
req.Request = &svcv1alpha1.GrantRequest_ResourceDetails{
ResourceDetails: &rbacapi.ResourceDetails{
- ResourceGroup: o.ResourceGroup,
- ResourceType: o.ResourceType,
- ResourceName: o.ResourceName,
- Verbs: o.Verbs,
+ ResourceType: o.ResourceType,
+ ResourceName: o.ResourceName,
+ Verbs: o.Verbs,
},
}
} else {
diff --git a/internal/cli/cmd/revoke/revoke.go b/internal/cli/cmd/revoke/revoke.go
index 3ca4bd59f..b7b0dfd0c 100644
--- a/internal/cli/cmd/revoke/revoke.go
+++ b/internal/cli/cmd/revoke/revoke.go
@@ -27,15 +27,14 @@ type revokeOptions struct {
Config config.CLIConfig
ClientOptions client.Options
- Project string
- Role string
- Subs []string
- Emails []string
- Groups []string
- ResourceGroup string
- ResourceType string
- ResourceName string
- Verbs []string
+ Project string
+ Role string
+ Subs []string
+ Emails []string
+ Groups []string
+ ResourceType string
+ ResourceName string
+ Verbs []string
}
func NewCommand(cfg config.CLIConfig, streams genericiooptions.IOStreams) *cobra.Command {
@@ -99,7 +98,6 @@ func (o *revokeOptions) addFlags(cmd *cobra.Command) {
option.Subs(cmd.Flags(), &o.Subs, "The sub claim of a user to have the role revoked.")
option.Emails(cmd.Flags(), &o.Emails, "The email address of a user to have the role revoked.")
option.Groups(cmd.Flags(), &o.Groups, "A group to have the role revoked.")
- option.ResourceGroup(cmd.Flags(), &o.ResourceGroup, "The group of resources to revoke permissions for.")
option.ResourceType(cmd.Flags(), &o.ResourceType, "A type of resource to revoke permissions for.")
option.ResourceName(cmd.Flags(), &o.ResourceName, "The name of a resource to revoke permissions for.")
option.Verbs(cmd.Flags(), &o.Verbs, "A verb to revoke on the resource.")
@@ -113,20 +111,16 @@ func (o *revokeOptions) addFlags(cmd *cobra.Command) {
option.SubFlag,
option.EmailFlag,
option.GroupFlag,
- option.ResourceGroupFlag,
+ option.ResourceTypeFlag,
)
// You can't revoke a role from users and revoke permissions from a role at
// the same time.
- cmd.MarkFlagsMutuallyExclusive(option.SubFlag, option.ResourceGroupFlag)
- cmd.MarkFlagsMutuallyExclusive(option.EmailFlag, option.ResourceGroupFlag)
- cmd.MarkFlagsMutuallyExclusive(option.GroupFlag, option.ResourceGroupFlag)
+ cmd.MarkFlagsMutuallyExclusive(option.SubFlag, option.ResourceTypeFlag)
+ cmd.MarkFlagsMutuallyExclusive(option.EmailFlag, option.ResourceTypeFlag)
+ cmd.MarkFlagsMutuallyExclusive(option.GroupFlag, option.ResourceTypeFlag)
- cmd.MarkFlagsRequiredTogether(
- option.ResourceGroupFlag,
- option.ResourceTypeFlag,
- option.VerbFlag,
- )
+ cmd.MarkFlagsRequiredTogether(option.ResourceTypeFlag, option.VerbFlag)
}
// validate performs validation of the options. If the options are invalid, an
@@ -155,15 +149,12 @@ func (o *revokeOptions) run(ctx context.Context) error {
Project: o.Project,
Role: o.Role,
}
- // Note: Don't test if ResourceGroup is empty, because "" is a legitimate
- // value.
if o.ResourceType != "" {
req.Request = &svcv1alpha1.RevokeRequest_ResourceDetails{
ResourceDetails: &rbacapi.ResourceDetails{
- ResourceGroup: o.ResourceGroup,
- ResourceType: o.ResourceType,
- ResourceName: o.ResourceName,
- Verbs: o.Verbs,
+ ResourceType: o.ResourceType,
+ ResourceName: o.ResourceName,
+ Verbs: o.Verbs,
},
}
} else {
diff --git a/internal/cli/option/flag.go b/internal/cli/option/flag.go
index 47ad13e1d..3b2786902 100644
--- a/internal/cli/option/flag.go
+++ b/internal/cli/option/flag.go
@@ -85,9 +85,6 @@ const (
// RepoURLFlag is the flag name for the repo-url flag.
RepoURLFlag = "repo-url"
- // ResourceGroupFlag is the flag name for the resource-group flag.
- ResourceGroupFlag = "resource-group"
-
// ResourceNameFlag is the flag name for the resource-name flag.
ResourceNameFlag = "resource-name"
@@ -255,11 +252,6 @@ func Regex(fs *pflag.FlagSet, regex *bool, usage string) {
fs.BoolVar(regex, RegexFlag, false, usage)
}
-// ResourceGroup adds the ResourceGroupFlag to the provided flag set.
-func ResourceGroup(fs *pflag.FlagSet, resourceGroup *string, usage string) {
- fs.StringVar(resourceGroup, ResourceGroupFlag, "", usage)
-}
-
// ResourceName adds the ResourceNameFlag to the provided flag set.
func ResourceName(fs *pflag.FlagSet, resourceName *string, usage string) {
fs.StringVar(resourceName, ResourceNameFlag, "", usage)
diff --git a/internal/controller/management/projects/projects.go b/internal/controller/management/projects/projects.go
index 2985b7b3c..a426698bb 100644
--- a/internal/controller/management/projects/projects.go
+++ b/internal/controller/management/projects/projects.go
@@ -8,7 +8,7 @@ import (
log "github.com/sirupsen/logrus"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
- apierrors "k8s.io/apimachinery/pkg/api/errors"
+ kubeerr "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/ptr"
@@ -19,8 +19,10 @@ import (
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/predicate"
+ rbacapi "github.com/akuity/kargo/api/rbac/v1alpha1"
kargoapi "github.com/akuity/kargo/api/v1alpha1"
"github.com/akuity/kargo/internal/controller"
+ rolloutsapi "github.com/akuity/kargo/internal/controller/rollouts/api/v1alpha1"
"github.com/akuity/kargo/internal/kubeclient"
"github.com/akuity/kargo/internal/logging"
)
@@ -83,7 +85,21 @@ type reconciler struct {
...client.UpdateOption,
) error
- ensureProjectAdminPermissionsFn func(context.Context, *kargoapi.Project) error
+ ensureAPIAdminPermissionsFn func(context.Context, *kargoapi.Project) error
+
+ ensureDefaultProjectRolesFn func(context.Context, *kargoapi.Project) error
+
+ createServiceAccountFn func(
+ context.Context,
+ client.Object,
+ ...client.CreateOption,
+ ) error
+
+ createRoleFn func(
+ context.Context,
+ client.Object,
+ ...client.CreateOption,
+ ) error
createRoleBindingFn func(
context.Context,
@@ -136,7 +152,10 @@ func newReconciler(kubeClient client.Client, cfg ReconcilerConfig) *reconciler {
r.getNamespaceFn = r.client.Get
r.createNamespaceFn = r.client.Create
r.updateNamespaceFn = r.client.Update
- r.ensureProjectAdminPermissionsFn = r.ensureProjectAdminPermissions
+ r.ensureAPIAdminPermissionsFn = r.ensureAPIAdminPermissions
+ r.ensureDefaultProjectRolesFn = r.ensureDefaultProjectRoles
+ r.createServiceAccountFn = r.client.Create
+ r.createRoleFn = r.client.Create
r.createRoleBindingFn = r.client.Create
r.deleteRoleBindingFn = r.client.Delete
r.ensureV06CompatibilityLabelFn = kargoapi.EnsureV06CompatibilityLabel
@@ -213,10 +232,14 @@ func (r *reconciler) syncProject(
return status, fmt.Errorf("error ensuring namespace: %w", err)
}
- if err = r.ensureProjectAdminPermissionsFn(ctx, project); err != nil {
+ if err = r.ensureAPIAdminPermissionsFn(ctx, project); err != nil {
return status, fmt.Errorf("error ensuring project admin permissions: %w", err)
}
+ if err = r.ensureDefaultProjectRolesFn(ctx, project); err != nil {
+ return status, fmt.Errorf("error ensuring default project roles: %w", err)
+ }
+
status.Phase = kargoapi.ProjectPhaseReady
return status, nil
}
@@ -273,7 +296,7 @@ func (r *reconciler) ensureNamespace(
project.Name,
)
}
- if !apierrors.IsNotFound(err) {
+ if !kubeerr.IsNotFound(err) {
return status, fmt.Errorf("error getting namespace %q: %w", project.Name, err)
}
@@ -302,7 +325,7 @@ func (r *reconciler) ensureNamespace(
return status, nil
}
-func (r *reconciler) ensureProjectAdminPermissions(
+func (r *reconciler) ensureAPIAdminPermissions(
ctx context.Context,
project *kargoapi.Project,
) error {
@@ -338,18 +361,19 @@ func (r *reconciler) ensureProjectAdminPermissions(
},
},
}
- if err := r.createRoleBindingFn(ctx, roleBinding); apierrors.IsAlreadyExists(err) {
- logger.Debug("role binding already exists in project namespace")
- } else if err != nil {
+ if err := r.createRoleBindingFn(ctx, roleBinding); err != nil {
+ if kubeerr.IsAlreadyExists(err) {
+ logger.Debug("RoleBinding already exists in project namespace")
+ return nil
+ }
return fmt.Errorf(
- "error creating role binding %q in project namespace %q: %w",
+ "error creating RoleBinding %q in project namespace %q: %w",
roleBinding.Name,
project.Name,
err,
)
- } else {
- logger.Debug("granted API server and kargo-admin project admin permissions")
}
+ logger.Debug("granted API server and kargo-admin project admin permissions")
// Delete legacy role binding if it exists
const legacyRoleBindingName = "kargo-api-server-manage-project-secrets"
@@ -361,7 +385,7 @@ func (r *reconciler) ensureProjectAdminPermissions(
Name: legacyRoleBindingName,
},
},
- ); apierrors.IsNotFound(err) {
+ ); kubeerr.IsNotFound(err) {
logger.Debug("legacy project admin role binding does not exist")
} else if err != nil {
return fmt.Errorf(
@@ -383,6 +407,197 @@ func (r *reconciler) ensureProjectAdminPermissions(
return nil
}
+func (r *reconciler) ensureDefaultProjectRoles(
+ ctx context.Context,
+ project *kargoapi.Project,
+) error {
+ logger := logging.LoggerFromContext(ctx).WithFields(log.Fields{
+ "project": project.Name,
+ "name": project.Name,
+ "namespace": project.Name,
+ })
+
+ const adminRoleName = "kargo-admin"
+ const viewerRoleName = "kargo-viewer"
+ allRoles := []string{adminRoleName, viewerRoleName}
+
+ for _, saName := range allRoles {
+ saLogger := logger.WithField("serviceAccount", saName)
+ if err := r.createServiceAccountFn(
+ ctx,
+ &corev1.ServiceAccount{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: saName,
+ Namespace: project.Name,
+ Annotations: map[string]string{
+ rbacapi.AnnotationKeyManaged: rbacapi.AnnotationValueTrue,
+ },
+ },
+ },
+ ); err != nil {
+ if kubeerr.IsAlreadyExists(err) {
+ saLogger.Debug("ServiceAccount already exists in project namespace")
+ continue
+ }
+ return fmt.Errorf(
+ "error creating ServiceAccount %q in project namespace %q: %w",
+ saName,
+ project.Name,
+ err,
+ )
+ }
+ }
+
+ roles := []*rbacv1.Role{
+ {
+ ObjectMeta: metav1.ObjectMeta{
+ Name: adminRoleName,
+ Namespace: project.Name,
+ Annotations: map[string]string{
+ rbacapi.AnnotationKeyManaged: rbacapi.AnnotationValueTrue,
+ },
+ },
+ Rules: []rbacv1.PolicyRule{
+ { // For viewing events; no need to create, edit, or delete them
+ APIGroups: []string{""},
+ Resources: []string{"events"},
+ Verbs: []string{"get", "list", "watch"},
+ },
+ { // For managing project-level access and credentials
+ APIGroups: []string{""},
+ Resources: []string{"secrets", "serviceaccounts"},
+ Verbs: []string{"*"},
+ },
+ { // For managing project-level access
+ APIGroups: []string{rbacv1.SchemeGroupVersion.Group},
+ Resources: []string{"rolebindings", "roles"},
+ Verbs: []string{"*"},
+ },
+ { // Full access to all mutable Kargo resource types
+ APIGroups: []string{kargoapi.GroupVersion.Group},
+ Resources: []string{"freights", "stages", "warehouses"},
+ Verbs: []string{"*"},
+ },
+ { // Promote permission on all stages
+ APIGroups: []string{kargoapi.GroupVersion.Group},
+ Resources: []string{"stages"},
+ Verbs: []string{"promote"},
+ },
+ { // Nearly full access to all Promotions, but they are immutable
+ APIGroups: []string{kargoapi.GroupVersion.Group},
+ Resources: []string{"promotions"},
+ Verbs: []string{"create", "delete", "get", "list", "watch"},
+ },
+ { // Manual approvals involve patching Freight status
+ APIGroups: []string{kargoapi.GroupVersion.Group},
+ Resources: []string{"freights/status"},
+ Verbs: []string{"patch"},
+ },
+ {
+ // View and delete AnalysisRuns
+ APIGroups: []string{rolloutsapi.GroupVersion.Group},
+ Resources: []string{"analysisruns"},
+ Verbs: []string{"delete", "get", "list", "watch"},
+ },
+ { // Full access to AnalysisTemplates
+ APIGroups: []string{rolloutsapi.GroupVersion.Group},
+ Resources: []string{"analysistemplates"},
+ Verbs: []string{"*"},
+ },
+ },
+ },
+ {
+ ObjectMeta: metav1.ObjectMeta{
+ Name: viewerRoleName,
+ Namespace: project.Name,
+ Annotations: map[string]string{
+ rbacapi.AnnotationKeyManaged: rbacapi.AnnotationValueTrue,
+ },
+ },
+ Rules: []rbacv1.PolicyRule{
+ {
+ APIGroups: []string{""},
+ Resources: []string{"events", "serviceaccounts"},
+ Verbs: []string{"get", "list", "watch"},
+ },
+ {
+ APIGroups: []string{rbacv1.SchemeGroupVersion.Group},
+ Resources: []string{"rolebindings", "roles"},
+ Verbs: []string{"get", "list", "watch"},
+ },
+ {
+ APIGroups: []string{kargoapi.GroupVersion.Group},
+ Resources: []string{"freights", "promotions", "stages", "warehouses"},
+ Verbs: []string{"get", "list", "watch"},
+ },
+ {
+ APIGroups: []string{rolloutsapi.GroupVersion.Group},
+ Resources: []string{"analysisruns", "analysistemplates"},
+ Verbs: []string{"get", "list", "watch"},
+ },
+ },
+ },
+ }
+ for _, role := range roles {
+ roleLogger := logger.WithField("role", role.Name)
+ if err := r.createRoleFn(ctx, role); err != nil {
+ if kubeerr.IsAlreadyExists(err) {
+ roleLogger.Debug("Role already exists in project namespace")
+ continue
+ }
+ return fmt.Errorf(
+ "error creating Role %q in project namespace %q: %w",
+ role.Name, project.Name, err,
+ )
+ }
+ roleLogger.Debugf(
+ "created Role %q in project namespace %q", role.Name, project.Name,
+ )
+ }
+
+ for _, rbName := range allRoles {
+ rbLogger := logger.WithField("roleBinding", rbName)
+ if err := r.createRoleBindingFn(
+ ctx,
+ &rbacv1.RoleBinding{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: rbName,
+ Namespace: project.Name,
+ Annotations: map[string]string{
+ rbacapi.AnnotationKeyManaged: rbacapi.AnnotationValueTrue,
+ },
+ },
+ RoleRef: rbacv1.RoleRef{
+ APIGroup: rbacv1.GroupName,
+ Kind: "Role",
+ Name: rbName,
+ },
+ Subjects: []rbacv1.Subject{
+ {
+ Kind: "ServiceAccount",
+ Name: rbName,
+ Namespace: project.Name,
+ },
+ },
+ },
+ ); err != nil {
+ if kubeerr.IsAlreadyExists(err) {
+ rbLogger.Debug("RoleBinding already exists in project namespace")
+ continue
+ }
+ return fmt.Errorf(
+ "error creating RoleBinding %q in project namespace %q: %w",
+ rbName, project.Name, err,
+ )
+ }
+ rbLogger.Debugf(
+ "created RoleBinding %q in project namespace %q", rbName, project.Name,
+ )
+ }
+
+ return nil
+}
+
func (r *reconciler) patchProjectStatus(
ctx context.Context,
project *kargoapi.Project,
diff --git a/internal/controller/management/projects/projects_test.go b/internal/controller/management/projects/projects_test.go
index f781e0360..f00d50763 100644
--- a/internal/controller/management/projects/projects_test.go
+++ b/internal/controller/management/projects/projects_test.go
@@ -30,7 +30,10 @@ func TestNewReconciler(t *testing.T) {
require.NotNil(t, r.getNamespaceFn)
require.NotNil(t, r.createNamespaceFn)
require.NotNil(t, r.updateNamespaceFn)
- require.NotNil(t, r.ensureProjectAdminPermissionsFn)
+ require.NotNil(t, r.ensureAPIAdminPermissionsFn)
+ require.NotNil(t, r.ensureDefaultProjectRolesFn)
+ require.NotNil(t, r.createServiceAccountFn)
+ require.NotNil(t, r.createRoleFn)
require.NotNil(t, r.createRoleBindingFn)
require.NotNil(t, r.deleteRoleBindingFn)
require.NotNil(t, r.ensureV06CompatibilityLabelFn)
@@ -225,7 +228,35 @@ func TestSyncProject(t *testing.T) {
) (kargoapi.ProjectStatus, error) {
return *project.Status.DeepCopy(), nil
},
- ensureProjectAdminPermissionsFn: func(
+ ensureAPIAdminPermissionsFn: func(
+ context.Context,
+ *kargoapi.Project,
+ ) error {
+ return errors.New("something went wrong")
+ },
+ },
+ assertions: func(t *testing.T, status kargoapi.ProjectStatus, err error) {
+ require.ErrorContains(t, err, "something went wrong")
+ // Still initializing because retry could succeed
+ require.Equal(t, kargoapi.ProjectPhaseInitializing, status.Phase)
+ },
+ },
+ {
+ name: "error ensuring default project roles",
+ reconciler: &reconciler{
+ ensureNamespaceFn: func(
+ _ context.Context,
+ project *kargoapi.Project,
+ ) (kargoapi.ProjectStatus, error) {
+ return *project.Status.DeepCopy(), nil
+ },
+ ensureAPIAdminPermissionsFn: func(
+ context.Context,
+ *kargoapi.Project,
+ ) error {
+ return nil
+ },
+ ensureDefaultProjectRolesFn: func(
context.Context,
*kargoapi.Project,
) error {
@@ -247,7 +278,13 @@ func TestSyncProject(t *testing.T) {
) (kargoapi.ProjectStatus, error) {
return *project.Status.DeepCopy(), nil
},
- ensureProjectAdminPermissionsFn: func(
+ ensureAPIAdminPermissionsFn: func(
+ context.Context,
+ *kargoapi.Project,
+ ) error {
+ return nil
+ },
+ ensureDefaultProjectRolesFn: func(
context.Context,
*kargoapi.Project,
) error {
@@ -508,7 +545,7 @@ func TestEnsureNamespace(t *testing.T) {
}
}
-func TestEnsureProjectAdminPermissions(t *testing.T) {
+func TestEnsureAPIAdminPermissions(t *testing.T) {
testCases := []struct {
name string
reconciler *reconciler
@@ -526,7 +563,7 @@ func TestEnsureProjectAdminPermissions(t *testing.T) {
},
},
assertions: func(t *testing.T, err error) {
- require.ErrorContains(t, err, "error creating role binding")
+ require.ErrorContains(t, err, "error creating RoleBinding")
require.ErrorContains(t, err, "something went wrong")
},
},
@@ -593,7 +630,125 @@ func TestEnsureProjectAdminPermissions(t *testing.T) {
t.Run(testCase.name, func(t *testing.T) {
testCase.assertions(
t,
- testCase.reconciler.ensureProjectAdminPermissions(
+ testCase.reconciler.ensureAPIAdminPermissions(
+ context.Background(),
+ &kargoapi.Project{},
+ ),
+ )
+ })
+ }
+}
+
+func TestEnsureDefaultProjectRoles(t *testing.T) {
+ testCases := []struct {
+ name string
+ reconciler *reconciler
+ assertions func(*testing.T, error)
+ }{
+ {
+ name: "error creating ServiceAccount",
+ reconciler: &reconciler{
+ createServiceAccountFn: func(
+ context.Context,
+ client.Object,
+ ...client.CreateOption,
+ ) error {
+ return errors.New("something went wrong")
+ },
+ },
+ assertions: func(t *testing.T, err error) {
+ require.ErrorContains(t, err, "error creating ServiceAccount")
+ require.ErrorContains(t, err, "something went wrong")
+ },
+ },
+ {
+ name: "error creating Role",
+ reconciler: &reconciler{
+ createServiceAccountFn: func(
+ context.Context,
+ client.Object,
+ ...client.CreateOption,
+ ) error {
+ return apierrors.NewAlreadyExists(schema.GroupResource{}, "")
+ },
+ createRoleFn: func(
+ context.Context,
+ client.Object,
+ ...client.CreateOption,
+ ) error {
+ return errors.New("something went wrong")
+ },
+ },
+ assertions: func(t *testing.T, err error) {
+ require.ErrorContains(t, err, "error creating Role")
+ require.ErrorContains(t, err, "something went wrong")
+ },
+ },
+ {
+ name: "error creating RoleBinding",
+ reconciler: &reconciler{
+ createServiceAccountFn: func(
+ context.Context,
+ client.Object,
+ ...client.CreateOption,
+ ) error {
+ return apierrors.NewAlreadyExists(schema.GroupResource{}, "")
+ },
+ createRoleFn: func(
+ context.Context,
+ client.Object,
+ ...client.CreateOption,
+ ) error {
+ return apierrors.NewAlreadyExists(schema.GroupResource{}, "")
+ },
+ createRoleBindingFn: func(
+ context.Context,
+ client.Object,
+ ...client.CreateOption,
+ ) error {
+ return errors.New("something went wrong")
+ },
+ },
+ assertions: func(t *testing.T, err error) {
+ require.ErrorContains(t, err, "error creating RoleBinding")
+ require.ErrorContains(t, err, "something went wrong")
+ },
+ },
+ {
+ name: "success",
+ reconciler: &reconciler{
+ createServiceAccountFn: func(
+ context.Context,
+ client.Object,
+ ...client.CreateOption,
+ ) error {
+ return nil
+ },
+ createRoleFn: func(
+ context.Context,
+ client.Object,
+ ...client.CreateOption,
+ ) error {
+ return nil
+ },
+ createRoleBindingFn: func(
+ context.Context,
+ client.Object,
+ ...client.CreateOption,
+ ) error {
+ return nil
+ },
+ },
+ assertions: func(t *testing.T, err error) {
+ require.NoError(t, err)
+ },
+ },
+ }
+ for _, testCase := range testCases {
+ t.Run(testCase.name, func(t *testing.T) {
+ testCase.assertions(
+ t,
+ testCase.reconciler.ensureDefaultProjectRoles(
context.Background(),
&kargoapi.Project{},
),
diff --git a/internal/controller/warehouses/git.go b/internal/controller/warehouses/git.go
index f82531139..b73019879 100644
--- a/internal/controller/warehouses/git.go
+++ b/internal/controller/warehouses/git.go
@@ -169,11 +169,10 @@ func (r *reconciler) selectTagAndCommitID(
baseCommit string,
) (string, string, error) {
+ var selectedTag, selectedCommit string
+ var err error
if sub.CommitSelectionStrategy == kargoapi.CommitSelectionStrategyNewestFromBranch {
- // In this case, there is nothing to do except return the commit ID at the
- // head of the branch unless there are includePaths/excludePaths configured to
- // handle.
- commit, err := r.getLastCommitIDFn(repo)
+ selectedCommit, err = r.getLastCommitIDFn(repo)
if err != nil {
return "", "",
fmt.Errorf("error determining commit ID at head of branch %q in git repo %q: %w",
@@ -182,111 +181,110 @@ func (r *reconciler) selectTagAndCommitID(
err,
)
}
- // In case includePaths/excludePaths filters are configured in a git subscription
- // below if clause deals with it. There is a special case - Warehouse has not
- // produced any Freight yet, this is sorted by creating Freight based on last
- // commit without applying filters.
- if (sub.IncludePaths != nil || sub.ExcludePaths != nil) && baseCommit != "" {
-
- // this shortcircuits to just return the last commit in case it is same as
- // baseCommit so we do not spam logs with errors of a valid not getting diffs
- // between baseCommit and HEAD (pointing to baseCommit in this case)
- if baseCommit == commit {
- return "", commit, nil
- }
- // getting actual diffPaths since baseCommit
- diffs, err := r.getDiffPathsSinceCommitIDFn(repo, baseCommit)
- if err != nil {
- return "", "",
- fmt.Errorf("error getting diffs since commit %q in git repo %q: %w",
- baseCommit,
- sub.RepoURL,
- err,
- )
- }
+ } else {
+ tags, err := r.listTagsFn(repo) // These are ordered newest to oldest
+ if err != nil {
+ return "", "", fmt.Errorf("error listing tags from git repo %q: %w", sub.RepoURL, err)
+ }
- matchesPathsFilters, err := matchesPathsFilters(sub.IncludePaths, sub.ExcludePaths, diffs)
- if err != nil {
- return "", "",
- fmt.Errorf("error checking includePaths/excludePaths match for commit %q for git repo %q: %w",
- commit,
- sub.RepoURL,
- err,
- )
+ // Narrow down the list of tags to those that are allowed and not ignored
+ allowRegex, err := regexp.Compile(sub.AllowTags)
+ if err != nil {
+ return "", "", fmt.Errorf("error compiling regular expression %q: %w", sub.AllowTags, err)
+ }
+ filteredTags := make([]string, 0, len(tags))
+ for _, tagName := range tags {
+ if allows(tagName, allowRegex) && !ignores(tagName, sub.IgnoreTags) {
+ filteredTags = append(filteredTags, tagName)
}
+ }
+ if len(filteredTags) == 0 {
+ return "", "", fmt.Errorf("found no applicable tags in repo %q", sub.RepoURL)
+ }
- if !matchesPathsFilters {
- return "", "",
- fmt.Errorf("commit %q not applicable due to includePaths/excludePaths configuration for repo %q",
- commit,
- sub.RepoURL,
- )
+ switch sub.CommitSelectionStrategy {
+ case kargoapi.CommitSelectionStrategyLexical:
+ selectedTag = selectLexicallyLastTag(filteredTags)
+ case kargoapi.CommitSelectionStrategyNewestTag:
+ selectedTag = filteredTags[0] // These are already ordered newest to oldest
+ case kargoapi.CommitSelectionStrategySemVer:
+ if selectedTag, err =
+ selectSemverTag(filteredTags, sub.SemverConstraint); err != nil {
+ return "", "", err
}
+ default:
+ return "", "", fmt.Errorf("unknown commit selection strategy %q", sub.CommitSelectionStrategy)
+ }
+ if selectedTag == "" {
+ return "", "", fmt.Errorf("found no applicable tags in repo %q", sub.RepoURL)
}
- return "", commit, nil
+ // Checkout the selected tag and return the commit ID
+ if err = r.checkoutTagFn(repo, selectedTag); err != nil {
+ return "", "", fmt.Errorf(
+ "error checking out tag %q from git repo %q: %w",
+ selectedTag,
+ sub.RepoURL,
+ err,
+ )
+ }
+ selectedCommit, err = r.getLastCommitIDFn(repo)
+ if err != nil {
+ return selectedTag, "", fmt.Errorf(
+ "error determining commit ID of tag %q in git repo %q: %w",
+ selectedTag,
+ sub.RepoURL,
+ err,
+ )
+ }
}
- tags, err := r.listTagsFn(repo) // These are ordered newest to oldest
- if err != nil {
- return "", "", fmt.Errorf("error listing tags from git repo %q: %w", sub.RepoURL, err)
+ // this shortcircuits to just return the last commit in case it is same as
+ // baseCommit so we do not spam logs with errors of a valid not getting diffs
+ // between baseCommit and HEAD (pointing to baseCommit in this case)
+ if baseCommit == selectedCommit {
+ return selectedTag, selectedCommit, nil
}
- // Narrow down the list of tags to those that are allowed and not ignored
- allowRegex, err := regexp.Compile(sub.AllowTags)
- if err != nil {
- return "", "", fmt.Errorf("error compiling regular expression %q: %w", sub.AllowTags, err)
- }
- filteredTags := make([]string, 0, len(tags))
- for _, tagName := range tags {
- if allows(tagName, allowRegex) && !ignores(tagName, sub.IgnoreTags) {
- filteredTags = append(filteredTags, tagName)
+ // In case includePaths/excludePaths filters are configured in a git subscription
+ // below if clause deals with it. There is a special case - Warehouse has not
+ // produced any Freight yet, this is sorted by creating Freight based on last
+ // commit without applying filters.
+ if (sub.IncludePaths != nil || sub.ExcludePaths != nil) && baseCommit != "" {
+
+ // getting actual diffPaths since baseCommit
+ diffs, err := r.getDiffPathsSinceCommitIDFn(repo, baseCommit)
+ if err != nil {
+ return selectedTag, "",
+ fmt.Errorf("error getting diffs since commit %q in git repo %q: %w",
+ baseCommit,
+ sub.RepoURL,
+ err,
+ )
}
- }
- if len(filteredTags) == 0 {
- return "", "", fmt.Errorf("found no applicable tags in repo %q", sub.RepoURL)
- }
- var selectedTag string
- switch sub.CommitSelectionStrategy {
- case kargoapi.CommitSelectionStrategyLexical:
- selectedTag = selectLexicallyLastTag(filteredTags)
- case kargoapi.CommitSelectionStrategyNewestTag:
- selectedTag = filteredTags[0] // These are already ordered newest to oldest
- case kargoapi.CommitSelectionStrategySemVer:
- if selectedTag, err =
- selectSemverTag(filteredTags, sub.SemverConstraint); err != nil {
- return "", "", err
+ matchesPathsFilters, err := matchesPathsFilters(sub.IncludePaths, sub.ExcludePaths, diffs)
+ if err != nil {
+ return selectedTag, "",
+ fmt.Errorf("error checking includePaths/excludePaths match for commit %q for git repo %q: %w",
+ selectedCommit,
+ sub.RepoURL,
+ err,
+ )
}
- default:
- return "", "", fmt.Errorf("unknown commit selection strategy %q", sub.CommitSelectionStrategy)
- }
- if selectedTag == "" {
- return "", "", fmt.Errorf("found no applicable tags in repo %q", sub.RepoURL)
- }
- // Checkout the selected tag and return the commit ID
- if err = r.checkoutTagFn(repo, selectedTag); err != nil {
- return "", "", fmt.Errorf(
- "error checking out tag %q from git repo %q: %w",
- selectedTag,
- sub.RepoURL,
- err,
- )
+ if !matchesPathsFilters {
+ return selectedTag, "",
+ fmt.Errorf("commit %q not applicable due to includePaths/excludePaths configuration for repo %q",
+ selectedCommit,
+ sub.RepoURL,
+ )
+ }
}
- commit, err := r.getLastCommitIDFn(repo)
- if err != nil {
- return "", "", fmt.Errorf(
- "error determining commit ID of tag %q in git repo %q: %w",
- selectedTag,
- sub.RepoURL,
- err,
- )
- }
- return selectedTag, commit, nil
+ return selectedTag, selectedCommit, nil
}
// allows returns true if the given tag name matches the given regular
diff --git a/internal/controller/warehouses/git_test.go b/internal/controller/warehouses/git_test.go
index 3883a1dab..9a4fe947c 100644
--- a/internal/controller/warehouses/git_test.go
+++ b/internal/controller/warehouses/git_test.go
@@ -177,6 +177,7 @@ func TestSelectCommitID(t *testing.T) {
name string
sub kargoapi.GitSubscription
reconciler *reconciler
+ baseCommit string
assertions func(t *testing.T, tag string, commit string, err error)
}{
{
@@ -226,7 +227,7 @@ func TestSelectCommitID(t *testing.T) {
},
assertions: func(t *testing.T, _, _ string, err error) {
require.ErrorContains(
- t, err, `error getting diffs since commit "sha" in git repo "":`,
+ t, err, `error getting diffs since commit "dummyBase" in git repo "":`,
)
require.ErrorContains(t, err, "something went wrong")
},
@@ -416,6 +417,33 @@ func TestSelectCommitID(t *testing.T) {
require.Equal(t, "fake-commit", commit)
},
},
+ {
+ name: "newest tag error due to path filters configuration",
+ sub: kargoapi.GitSubscription{
+ CommitSelectionStrategy: kargoapi.CommitSelectionStrategyNewestTag,
+ IncludePaths: []string{regexpPrefix + "^.*third_path_to_a/file$"},
+ },
+ reconciler: &reconciler{
+ listTagsFn: func(git.Repo) ([]string, error) {
+ return []string{"abc", "xyz"}, nil
+ },
+ checkoutTagFn: func(git.Repo, string) error {
+ return nil
+ },
+ getLastCommitIDFn: func(git.Repo) (string, error) {
+ return "fake-commit", nil
+ },
+ getDiffPathsSinceCommitIDFn: func(git.Repo, string) ([]string, error) {
+ return []string{"first_path_to_a/file", "second_path_to_a/file"}, nil
+ },
+ },
+ assertions: func(t *testing.T, tag, commit string, err error) {
+ require.Equal(t, "abc", tag)
+ require.ErrorContains(t, err, "commit \"fake-commit\" not applicable due to ")
+ require.ErrorContains(t, err, "includePaths/excludePaths configuration for repo")
+ require.Equal(t, "", commit)
+ },
+ },
{
name: "semver error selecting tag",
sub: kargoapi.GitSubscription{
@@ -459,7 +487,7 @@ func TestSelectCommitID(t *testing.T) {
tag, commit, err := testCase.reconciler.selectTagAndCommitID(
nil,
testCase.sub,
- "sha",
+ "dummyBase",
)
testCase.assertions(t, tag, commit, err)
})
diff --git a/ui/public/favicon.ico b/ui/public/favicon.ico
index 98474dd99..c6bfa5edc 100644
Binary files a/ui/public/favicon.ico and b/ui/public/favicon.ico differ
diff --git a/ui/public/kargo-icon.png b/ui/public/kargo-icon.png
index 9b188762d..7210db8f7 100644
Binary files a/ui/public/kargo-icon.png and b/ui/public/kargo-icon.png differ
diff --git a/ui/src/app.tsx b/ui/src/app.tsx
index 5e75ba152..89f3cb4e8 100644
--- a/ui/src/app.tsx
+++ b/ui/src/app.tsx
@@ -36,6 +36,7 @@ export const App = () => (
element={}
/>
} />
+ } />
} />
} />
diff --git a/ui/src/config/paths.ts b/ui/src/config/paths.ts
index eff01d18a..ac5313c2c 100644
--- a/ui/src/config/paths.ts
+++ b/ui/src/config/paths.ts
@@ -5,6 +5,7 @@ export const paths = {
projectCredentials: '/project/:name/credentials',
projectAnalysisTemplates: '/project/:name/analysis-templates',
projectEvents: '/project/:name/events',
+ projectRoles: '/project/:name/roles',
stage: '/project/:name/stage/:stageName',
freight: '/project/:name/freight/:freightName',
diff --git a/ui/src/features/common/confirm-modal/confirm-modal.tsx b/ui/src/features/common/confirm-modal/confirm-modal.tsx
index cf62813a7..9b6f1b1b7 100644
--- a/ui/src/features/common/confirm-modal/confirm-modal.tsx
+++ b/ui/src/features/common/confirm-modal/confirm-modal.tsx
@@ -1,10 +1,9 @@
-import { Modal } from 'antd';
-
-import { ModalProps } from '../modal/use-modal';
+import { Modal, ModalFuncProps } from 'antd';
export interface ConfirmProps {
title: string | React.ReactNode;
onOk: () => void;
+ hide: () => void;
content?: string | React.ReactNode;
}
@@ -13,15 +12,23 @@ export const ConfirmModal = ({
title = 'Are you sure?',
content,
hide,
- visible
-}: ConfirmProps & ModalProps) => {
+ visible,
+ ...props
+}: ConfirmProps & ModalFuncProps) => {
const onConfirm = () => {
onOk();
hide();
};
return (
-
+
{content}
);
diff --git a/ui/src/features/common/form/field-container.tsx b/ui/src/features/common/form/field-container.tsx
index f387c05dc..50edeed37 100644
--- a/ui/src/features/common/form/field-container.tsx
+++ b/ui/src/features/common/form/field-container.tsx
@@ -11,18 +11,20 @@ interface Props extends UseControllerProps {
children: (props: UseControllerReturn) => React.ReactNode;
label?: string;
formItemOptions?: Omit;
+ className?: string;
}
export const FieldContainer = ({
children,
label,
formItemOptions,
+ className,
...props
}: Props) => {
const controller = useController(props);
return (
-