feat: support project-level Mattermost integration (client-go v1)#268
Conversation
1d25ab6 to
cd200fd
Compare
.gitignore
Outdated
| # Local testing | ||
| gitlab/ | ||
| docker-compose.yml | ||
| test-resources/ |
There was a problem hiding this comment.
I wouldn't consider these files "common" in the sense that everyone uses them for his local dev setup. I case of custom files you want to ignore you can add them to .git/info/exclude which acts as a kind of local gitignore that is not committed.
|
|
||
| // Send notifications for broken pipelines. | ||
| // +optional | ||
| NotifyOnlyBrokenPipelines *bool `json:"notify_only_broken_pipelines,omitempty"` |
There was a problem hiding this comment.
General rule for API property naming: JSON names should not use underscores and use camel case for the first letter: i.e. projectId instead of projectID or project_id. Does not apply to Go field names: There it is ProjectID instead of ProjectId.
| NotifyOnlyBrokenPipelines *bool `json:"notify_only_broken_pipelines,omitempty"` | |
| NotifyOnlyBrokenPipelines *bool `json:"notifyOnlyBrokenPipelines,omitempty"` |
| func (e *external) Update(ctx context.Context, mg resource.Managed) (managed.ExternalUpdate, error) { | ||
| cr, ok := mg.(*v1alpha1.IntegrationMattermost) | ||
| if !ok { | ||
| return managed.ExternalUpdate{}, errors.New(errNotIntegrationMattermost) | ||
| } | ||
|
|
||
| cr.Status.SetConditions(xpv1.Creating()) | ||
|
|
||
| // Call GitLab Mattermost service API | ||
| _, _, err := e.client.SetMattermostService( | ||
| *cr.Spec.ForProvider.ProjectID, | ||
| projects.GenerateSetMattermostServiceOptions(&cr.Spec.ForProvider), | ||
| gitlab.WithContext(ctx)) | ||
| if err != nil { | ||
| return managed.ExternalUpdate{}, errors.Wrap(err, errUpdateFailed) | ||
| } | ||
|
|
||
| return managed.ExternalUpdate{}, nil | ||
| } |
There was a problem hiding this comment.
Create and Update functions are basically identical so we can move the call to SetMatterMost() into an extra function and just call it in both.
| if err != nil { | ||
| if clients.IsResponseNotFound(res) { | ||
| return managed.ExternalObservation{ResourceExists: false}, nil | ||
| } | ||
| return managed.ExternalObservation{}, errors.Wrap(err, errGetFailed) | ||
| } |
There was a problem hiding this comment.
Can be simplified. errors.Wrap() returns nil for nil errors.
Also, false is default value for boolean fields, so ResourceExists: false is implied.
| if err != nil { | |
| if clients.IsResponseNotFound(res) { | |
| return managed.ExternalObservation{ResourceExists: false}, nil | |
| } | |
| return managed.ExternalObservation{}, errors.Wrap(err, errGetFailed) | |
| } | |
| if resource.Ignore(clients.IsResponseNotFound, err) != nil { | |
| return managed.ExternalObservation{}, errors.Wrap(err, errGetFailed) | |
| } |
pkg/namespaced/controller/projects/integrationmattermost/controller.go
Outdated
Show resolved
Hide resolved
pkg/namespaced/controller/projects/integrationmattermost/controller.go
Outdated
Show resolved
Hide resolved
cd200fd to
a221715
Compare
apis/common/v1alpha1/integration.go
Outdated
| ID int64 `json:"id"` | ||
| Title string `json:"title"` | ||
| Slug string `json:"slug"` | ||
| CreatedAt *metav1.Time `json:"createdAt,omitempty"` | ||
| UpdatedAt *metav1.Time `json:"updatedAt,omitempty"` | ||
| Active bool `json:"active"` | ||
| AlertEvents bool `json:"alertEvents"` | ||
| CommitEvents bool `json:"commitEvents"` | ||
| ConfidentialIssuesEvents bool `json:"confidentialIssuesEvents"` | ||
| ConfidentialNoteEvents bool `json:"confidentialNoteEvents"` | ||
| DeploymentEvents bool `json:"deploymentEvents"` | ||
| GroupConfidentialMentionEvents bool `json:"groupConfidentialMentionEvents"` | ||
| GroupMentionEvents bool `json:"groupMentionEvents"` | ||
| IncidentEvents bool `json:"incidentEvents"` | ||
| IssuesEvents bool `json:"issuesEvents"` | ||
| JobEvents bool `json:"jobEvents"` | ||
| MergeRequestsEvents bool `json:"mergeRequestsEvents"` | ||
| NoteEvents bool `json:"noteEvents"` | ||
| PipelineEvents bool `json:"pipelineEvents"` | ||
| PushEvents bool `json:"pushEvents"` | ||
| TagPushEvents bool `json:"tagPushEvents"` | ||
| VulnerabilityEvents bool `json:"vulnerabilityEvents"` | ||
| WikiPageEvents bool `json:"wikiPageEvents"` | ||
| CommentOnEventEnabled bool `json:"commentOnEventEnabled"` | ||
| Inherited bool `json:"inherited"` |
There was a problem hiding this comment.
Not sure, but I think they should be optional. Otherwise we might run into an error at the end of the reconcilation when the status update fails due to a missing field.
a221715 to
f1931d4
Compare
|
@Vadims-U you probably need to merge the latest changes i did to the branch into yours. But I would recommend waiting for my MR to hit master. Also I could imagine that you receive a lot of Merge conflicts because i had to merge all commits into one. Just as a heads up! sorry! |
Hi! Nothing to worry about. Thanks for the heads up - I’ll wait for the MR to land in master 👍 |
|
@Vadims-U now that my PR has landed would you be so kind and update your PR? |
f1931d4 to
a55949e
Compare
|
@Vadims-U I think you forgot to sign your Commit would you be so kind and do this? :) |
60c4235 to
3a83f8f
Compare
| // PushEventsBranchFilter triggers hook on push events for matching branches only. | ||
| // +optional | ||
| PushEventsBranchFilter *string `json:"pushEventsBranch_filter,omitempty"` | ||
| PushEventsBranchFilter *string `json:"pushEventsBranchFilter,omitempty"` |
There was a problem hiding this comment.
Is this change related to this PR? If not it is a breaking change, which we should avoid.
apis/common/v1alpha1/integration.go
Outdated
| type CommonIntegrationParameters struct { | ||
| // Send notifications for broken pipelines. | ||
| // +optional | ||
| NotifyOnlyBrokenPipelines *bool `json:"notify_only_broken_pipelines,omitempty"` |
There was a problem hiding this comment.
Please replace underscores with camel case.
| @@ -93,7 +93,7 @@ type RunnerProject struct { | |||
| // PathWithNamespace is the full path of the project including its namespace. | |||
| // This follows the format "namespace/project-path" and is used in URLs. | |||
| // +optional | |||
| PathWithNamespace string `json:"path_with_namespace"` | |||
| PathWithNamespace string `json:"pathWithNamespace"` | |||
There was a problem hiding this comment.
I think this is a breaking change as well.
pkg/common/project_integration.go
Outdated
| func ptrBool(v bool) *bool { return &v } | ||
| func ptrInt64(v int64) *int64 { return &v } | ||
| func ptrString(v string) *string { return &v } |
There was a problem hiding this comment.
Please use the ptr package instead: https://pkg.go.dev/k8s.io/utils/ptr
pkg/common/helper.go
Outdated
| // Int64PtrToInt64Ptr is a no-op now that CRDs use int64 | ||
| // Kept for API compatibility | ||
| func Int64PtrToInt64Ptr(v *int64) *int64 { | ||
| return v | ||
| } | ||
|
|
||
| // IntToInt64 converts int to int64 | ||
| func IntToInt64(v int) int64 { | ||
| return int64(v) | ||
| } | ||
|
|
||
| // Int64ToInt converts int64 to int | ||
| func Int64ToInt(v int64) int { | ||
| return int(v) | ||
| } |
There was a problem hiding this comment.
Do we really need this functions?
There was a problem hiding this comment.
This should be removed as soon as Main is rebased into this branch
|
@Vadims-U would you be so kind and merge master into your branch so everythings up to date? |
Migrate all CRD field types from *int to *int64 to align with GitLab client-go v1.0 API which uses int64 for all ID fields. This is NOT a breaking change for users as YAML/JSON does not distinguish between int and int64 - both serialize as numbers. Changes: - Update all API type definitions in apis/namespaced/ - Update all API type definitions in apis/cluster/ - Update helper functions: IsIntEqualToIntPtr -> IsInt64EqualToInt64Ptr - Update client code to remove unnecessary int() casts - Update controller code to work with int64 types directly - Regenerate deepcopy code for all API packages - Regenerate cluster-scoped files from namespaced templates Benefits: - Eliminates hundreds of manual type conversions - Direct compatibility with GitLab API types - More maintainable codebase - Prevents potential overflow issues with large IDs Files affected: - All *_types.go files in apis/namespaced/ and apis/cluster/ - Client implementations in pkg/namespaced/clients/ - Controller implementations in pkg/namespaced/controller/ - Generated cluster-scoped equivalents Related to GitLab client-go v1.0 migration. Signed-off-by: Henry Sachs <henry.sachs@deutschebahn.com>
Signed-off-by: BoxBoxJason <contact@boxboxjason.dev> Signed-off-by: Vadims-U <vadims.ubikins@accenture.com> # Conflicts: # apis/cluster/projects/v1alpha1/zz_register.go # apis/namespaced/projects/v1alpha1/register.go # pkg/cluster/controller/projects/zz_setup.go # pkg/namespaced/controller/projects/setup.go
Signed-off-by: Vadims-U <vadims.ubikins@accenture.com>
…lints Signed-off-by: Vadims-U <vadims.ubikins@accenture.com>
3a83f8f to
307d896
Compare
| keepN: | ||
| format: int64 | ||
| type: integer | ||
| name_regex: |
| keepN: | ||
| format: int64 | ||
| type: integer | ||
| name_regex: |
…lints Signed-off-by: Vadims-U <vadims.ubikins@accenture.com>
|
@Vadims-U the DCO is still failing so you need to sign off your commits please |
… behavior, full test suite, controller fixes Signed-off-by: Vadims-U <vadims.ubikins@accenture.com>
2c5241e to
2da3977
Compare
This PR completes and modernizes support for project‑level Mattermost integration, originally started in draft PR #261. The draft required updates to work with the new
gitlab-org/api/client-go v1SDK and recent provider changes.This PR:
int64),I recommend a squash merge to keep history clean.
Refs #250. Supersedes #261. Builds on top of #264.
What was done
Based on draft PR #261
The initial implementation was outdated and non‑functional. Main issues:
old client-go usage,
deprecated numeric types (
int→int64),incompatible with client-go v1.
This PR completes and modernizes that draft.
Changes in this PR
Updated to client-go v1 (using #264)
*int64,Completed implementation
finished controller logic,
fixed Create / Update logic
implemented reference resolution
Regenerated and aligned code
make generate,Tests & validation
Reconciliation behavior
Test environment & workflow
Environment:
kindcluster (Podman provider):make go.build && ./_output/bin/linux_amd64/provider --debugTest steps:
Create secret with PAT:
Apply
ProviderConfig:Apply
examples/projects/integration_mattermost.yaml.Verify parameters applied correctly.
Update CRD fields and confirm correct reconciliation.
Modify integration manually in GitLab and re‑verify.
Delete resource and confirm no‑op behavior.
Example manifest used during testing