diff --git a/components/backend/git/operations.go b/components/backend/git/operations.go index 69022267f..0b3c84e6d 100644 --- a/components/backend/git/operations.go +++ b/components/backend/git/operations.go @@ -114,7 +114,7 @@ func GetGitHubToken(ctx context.Context, k8sClient *kubernetes.Clientset, dynCli } // GetGitLabToken retrieves a GitLab Personal Access Token for a user -func GetGitLabToken(ctx context.Context, k8sClient *kubernetes.Clientset, project, userID string) (string, error) { +func GetGitLabToken(ctx context.Context, k8sClient kubernetes.Interface, project, userID string) (string, error) { if k8sClient == nil { log.Printf("Cannot read GitLab token: k8s client is nil") return "", fmt.Errorf("no GitLab credentials available. Please connect your GitLab account") @@ -817,40 +817,44 @@ func DetectPushError(repoURL, stderr, stdout string) error { // Check for authentication/permission errors if strings.Contains(combined, "403") || strings.Contains(combined, "forbidden") { - if provider == types.ProviderGitLab { - return fmt.Errorf("GitLab push failed: Insufficient permissions. Ensure your GitLab token has 'write_repository' scope. You can update your token by reconnecting your GitLab account with the required permissions") - } else if provider == types.ProviderGitHub { - return fmt.Errorf("GitHub push failed: Insufficient permissions. Check that your GitHub App installation has write access to this repository") + switch provider { + case types.ProviderGitLab: + return fmt.Errorf("GitLab push failed: insufficient permissions. Ensure your GitLab token has 'write_repository' scope. You can update your token by reconnecting your GitLab account with the required permissions") + case types.ProviderGitHub: + return fmt.Errorf("GitHub push failed: insufficient permissions. Check that your GitHub App installation has write access to this repository") + default: + return fmt.Errorf("push failed: insufficient permissions (403 Forbidden)") } - return fmt.Errorf("Push failed: Insufficient permissions (403 Forbidden)") } // Check for authentication failures if strings.Contains(combined, "401") || strings.Contains(combined, "unauthorized") || strings.Contains(combined, "authentication failed") { - if provider == types.ProviderGitLab { - return fmt.Errorf("GitLab push failed: Authentication failed. Your GitLab token may be invalid or expired. Please reconnect your GitLab account") - } else if provider == types.ProviderGitHub { - return fmt.Errorf("GitHub push failed: Authentication failed. Check your GitHub App installation") + switch provider { + case types.ProviderGitLab: + return fmt.Errorf("GitLab push failed: authentication failed. Your GitLab token may be invalid or expired. Please reconnect your GitLab account") + case types.ProviderGitHub: + return fmt.Errorf("GitHub push failed: authentication failed. Check your GitHub App installation") + default: + return fmt.Errorf("push failed: authentication failed (401 Unauthorized)") } - return fmt.Errorf("Push failed: Authentication failed (401 Unauthorized)") } // Check for network errors if strings.Contains(combined, "could not resolve host") || strings.Contains(combined, "connection refused") { - return fmt.Errorf("Push failed: Unable to connect to %s. Check network connectivity", extractHostFromURL(repoURL)) + return fmt.Errorf("push failed: unable to connect to %s. Check network connectivity", extractHostFromURL(repoURL)) } // Check for rate limiting if strings.Contains(combined, "429") || strings.Contains(combined, "rate limit") { if provider == types.ProviderGitLab { - return fmt.Errorf("GitLab push failed: Rate limit exceeded. Please wait a few minutes before retrying") + return fmt.Errorf("GitLab push failed: rate limit exceeded. Please wait a few minutes before retrying") } - return fmt.Errorf("Push failed: API rate limit exceeded. Please wait before retrying") + return fmt.Errorf("push failed: API rate limit exceeded. Please wait before retrying") } // Check for repository not found if strings.Contains(combined, "404") || strings.Contains(combined, "not found") || strings.Contains(combined, "repository not found") { - return fmt.Errorf("Push failed: Repository not found. Verify the repository URL: %s", repoURL) + return fmt.Errorf("push failed: repository not found. Verify the repository URL: %s", repoURL) } // Return original error if no pattern matched @@ -861,7 +865,7 @@ func DetectPushError(repoURL, stderr, stdout string) error { if len(errMsg) > 500 { errMsg = errMsg[:500] + "..." } - return fmt.Errorf("Push failed: %s", errMsg) + return fmt.Errorf("push failed: %s", errMsg) } // extractHostFromURL extracts the host from a git URL for error messages @@ -1491,13 +1495,8 @@ func validateGitLabPushAccess(ctx context.Context, repoURL, gitlabToken string) // GitLab access levels: 10=Guest, 20=Reporter, 30=Developer, 40=Maintainer, 50=Owner // Need at least Developer (30) to push - hasAccess := false - if projectInfo.Permissions.ProjectAccess != nil && projectInfo.Permissions.ProjectAccess.AccessLevel >= 30 { - hasAccess = true - } - if projectInfo.Permissions.GroupAccess != nil && projectInfo.Permissions.GroupAccess.AccessLevel >= 30 { - hasAccess = true - } + hasAccess := (projectInfo.Permissions.ProjectAccess != nil && projectInfo.Permissions.ProjectAccess.AccessLevel >= 30) || + (projectInfo.Permissions.GroupAccess != nil && projectInfo.Permissions.GroupAccess.AccessLevel >= 30) if !hasAccess { return fmt.Errorf("you don't have push access to %s. You need at least Developer (30) access level. Please check your permissions in GitLab", repoURL) diff --git a/components/backend/gitlab/connection.go b/components/backend/gitlab/connection.go index 1d6e924b5..0832e49c1 100644 --- a/components/backend/gitlab/connection.go +++ b/components/backend/gitlab/connection.go @@ -14,12 +14,12 @@ import ( // ConnectionManager handles GitLab connection operations type ConnectionManager struct { - clientset *kubernetes.Clientset + clientset kubernetes.Interface namespace string } // NewConnectionManager creates a new connection manager -func NewConnectionManager(clientset *kubernetes.Clientset, namespace string) *ConnectionManager { +func NewConnectionManager(clientset kubernetes.Interface, namespace string) *ConnectionManager { return &ConnectionManager{ clientset: clientset, namespace: namespace, diff --git a/components/backend/handlers/gitlab_auth.go b/components/backend/handlers/gitlab_auth.go index 193d2ab1b..1d8436909 100644 --- a/components/backend/handlers/gitlab_auth.go +++ b/components/backend/handlers/gitlab_auth.go @@ -85,10 +85,10 @@ func validateGitLabInput(instanceURL, token string) error { // Validate token contains only valid characters (alphanumeric and some special chars) // GitLab tokens use: a-z, A-Z, 0-9, -, _ for _, char := range token { - if !((char >= 'a' && char <= 'z') || - (char >= 'A' && char <= 'Z') || - (char >= '0' && char <= '9') || - char == '-' || char == '_') { + if (char < 'a' || char > 'z') && + (char < 'A' || char > 'Z') && + (char < '0' || char > '9') && + char != '-' && char != '_' { return fmt.Errorf("token contains invalid characters") } } diff --git a/components/backend/k8s/configmap.go b/components/backend/k8s/configmap.go index f5d791113..c97b69535 100644 --- a/components/backend/k8s/configmap.go +++ b/components/backend/k8s/configmap.go @@ -19,7 +19,7 @@ const ( ) // StoreGitLabConnection stores GitLab connection metadata in a ConfigMap -func StoreGitLabConnection(ctx context.Context, clientset *kubernetes.Clientset, namespace string, connection *types.GitLabConnection) error { +func StoreGitLabConnection(ctx context.Context, clientset kubernetes.Interface, namespace string, connection *types.GitLabConnection) error { configMapsClient := clientset.CoreV1().ConfigMaps(namespace) // Serialize connection to JSON @@ -68,7 +68,7 @@ func StoreGitLabConnection(ctx context.Context, clientset *kubernetes.Clientset, } // GetGitLabConnection retrieves GitLab connection metadata from a ConfigMap -func GetGitLabConnection(ctx context.Context, clientset *kubernetes.Clientset, namespace, userID string) (*types.GitLabConnection, error) { +func GetGitLabConnection(ctx context.Context, clientset kubernetes.Interface, namespace, userID string) (*types.GitLabConnection, error) { configMapsClient := clientset.CoreV1().ConfigMaps(namespace) configMap, err := configMapsClient.Get(ctx, GitLabConnectionsConfigMapName, metav1.GetOptions{}) @@ -93,7 +93,7 @@ func GetGitLabConnection(ctx context.Context, clientset *kubernetes.Clientset, n } // DeleteGitLabConnection removes GitLab connection metadata from a ConfigMap -func DeleteGitLabConnection(ctx context.Context, clientset *kubernetes.Clientset, namespace, userID string) error { +func DeleteGitLabConnection(ctx context.Context, clientset kubernetes.Interface, namespace, userID string) error { configMapsClient := clientset.CoreV1().ConfigMaps(namespace) configMap, err := configMapsClient.Get(ctx, GitLabConnectionsConfigMapName, metav1.GetOptions{}) @@ -119,7 +119,7 @@ func DeleteGitLabConnection(ctx context.Context, clientset *kubernetes.Clientset } // HasGitLabConnection checks if a user has a GitLab connection stored -func HasGitLabConnection(ctx context.Context, clientset *kubernetes.Clientset, namespace, userID string) (bool, error) { +func HasGitLabConnection(ctx context.Context, clientset kubernetes.Interface, namespace, userID string) (bool, error) { configMapsClient := clientset.CoreV1().ConfigMaps(namespace) configMap, err := configMapsClient.Get(ctx, GitLabConnectionsConfigMapName, metav1.GetOptions{}) @@ -135,7 +135,7 @@ func HasGitLabConnection(ctx context.Context, clientset *kubernetes.Clientset, n } // ListGitLabConnections retrieves all GitLab connections from a ConfigMap -func ListGitLabConnections(ctx context.Context, clientset *kubernetes.Clientset, namespace string) ([]*types.GitLabConnection, error) { +func ListGitLabConnections(ctx context.Context, clientset kubernetes.Interface, namespace string) ([]*types.GitLabConnection, error) { configMapsClient := clientset.CoreV1().ConfigMaps(namespace) configMap, err := configMapsClient.Get(ctx, GitLabConnectionsConfigMapName, metav1.GetOptions{}) diff --git a/components/backend/k8s/secrets.go b/components/backend/k8s/secrets.go index 80105915a..151fc0c5c 100644 --- a/components/backend/k8s/secrets.go +++ b/components/backend/k8s/secrets.go @@ -18,7 +18,7 @@ const ( // StoreGitLabToken stores a GitLab Personal Access Token in Kubernetes Secrets // Uses optimistic concurrency control with retry to handle concurrent updates -func StoreGitLabToken(ctx context.Context, clientset *kubernetes.Clientset, namespace, userID, token string) error { +func StoreGitLabToken(ctx context.Context, clientset kubernetes.Interface, namespace, userID, token string) error { secretsClient := clientset.CoreV1().Secrets(namespace) // Retry up to 3 times with exponential backoff @@ -88,7 +88,7 @@ func StoreGitLabToken(ctx context.Context, clientset *kubernetes.Clientset, name } // GetGitLabToken retrieves a GitLab Personal Access Token from Kubernetes Secrets -func GetGitLabToken(ctx context.Context, clientset *kubernetes.Clientset, namespace, userID string) (string, error) { +func GetGitLabToken(ctx context.Context, clientset kubernetes.Interface, namespace, userID string) (string, error) { secretsClient := clientset.CoreV1().Secrets(namespace) secret, err := secretsClient.Get(ctx, GitLabTokensSecretName, metav1.GetOptions{}) @@ -109,7 +109,7 @@ func GetGitLabToken(ctx context.Context, clientset *kubernetes.Clientset, namesp // DeleteGitLabToken removes a GitLab Personal Access Token from Kubernetes Secrets // Uses optimistic concurrency control with retry to handle concurrent updates -func DeleteGitLabToken(ctx context.Context, clientset *kubernetes.Clientset, namespace, userID string) error { +func DeleteGitLabToken(ctx context.Context, clientset kubernetes.Interface, namespace, userID string) error { secretsClient := clientset.CoreV1().Secrets(namespace) // Retry up to 3 times with exponential backoff @@ -155,7 +155,7 @@ func DeleteGitLabToken(ctx context.Context, clientset *kubernetes.Clientset, nam } // HasGitLabToken checks if a user has a GitLab token stored -func HasGitLabToken(ctx context.Context, clientset *kubernetes.Clientset, namespace, userID string) (bool, error) { +func HasGitLabToken(ctx context.Context, clientset kubernetes.Interface, namespace, userID string) (bool, error) { secretsClient := clientset.CoreV1().Secrets(namespace) secret, err := secretsClient.Get(ctx, GitLabTokensSecretName, metav1.GetOptions{}) diff --git a/components/backend/tests/integration/gitlab/gitlab_integration_test.go b/components/backend/tests/integration/gitlab/gitlab_integration_test.go index 3a26fec26..d638cea14 100644 --- a/components/backend/tests/integration/gitlab/gitlab_integration_test.go +++ b/components/backend/tests/integration/gitlab/gitlab_integration_test.go @@ -4,7 +4,6 @@ import ( "context" "os" "testing" - "time" "ambient-code-backend/git" "ambient-code-backend/gitlab" @@ -14,7 +13,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" ) @@ -48,8 +46,9 @@ func TestGitLabIntegrationEnd2End(t *testing.T) { result, err := gitlab.ValidateGitLabToken(ctx, gitlabToken, "https://gitlab.com") require.NoError(t, err, "Token validation should succeed") assert.True(t, result.Valid, "Token should be valid") - assert.NotEmpty(t, result.Username, "Username should be populated") - assert.NotEmpty(t, result.GitLabUserID, "GitLab user ID should be populated") + assert.NotNil(t, result.User, "User should be populated") + assert.NotEmpty(t, result.User.Username, "Username should be populated") + assert.NotZero(t, result.User.ID, "GitLab user ID should be populated") }) // Test token storage @@ -362,7 +361,7 @@ func TestGitLabClientLogging(t *testing.T) { client := gitlab.NewClient("https://gitlab.com/api/v4", token) // Make a simple API request - resp, err := client.GetCurrentUser(ctx) + resp, err := gitlab.GetCurrentUser(ctx, client) require.NoError(t, err, "API call should succeed") assert.NotNil(t, resp, "Response should not be nil") diff --git a/components/backend/types/common.go b/components/backend/types/common.go index 1d86bb2a1..930046f18 100644 --- a/components/backend/types/common.go +++ b/components/backend/types/common.go @@ -80,15 +80,17 @@ type FileContent struct { SHA string `json:"sha,omitempty"` } -// Helper functions for pointer types +// BoolPtr returns a pointer to the given bool value. func BoolPtr(b bool) *bool { return &b } +// StringPtr returns a pointer to the given string value. func StringPtr(s string) *string { return &s } +// IntPtr returns a pointer to the given int value. func IntPtr(i int) *int { return &i } diff --git a/components/backend/types/gitlab.go b/components/backend/types/gitlab.go index 78dfe3830..6532c71b5 100644 --- a/components/backend/types/gitlab.go +++ b/components/backend/types/gitlab.go @@ -11,7 +11,7 @@ type GitLabConnection struct { UpdatedAt time.Time `json:"updatedAt"` // Last connection update } -// GitLabRepository extends GitRepository for GitLab-specific attributes +// ParsedGitLabRepo extends GitRepository for GitLab-specific attributes. // Internal parsed representation (not persisted to CRD) type ParsedGitLabRepo struct { Host string // "gitlab.com" or "gitlab.example.com"