Skip to content
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.

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

Adding necessary field names to handle external IPs in connection secret #191

Merged
merged 6 commits into from
Aug 8, 2023

Conversation

wejdross
Copy link
Member

This PR is strictly connected with: vshn/appcat#46
I need to add fields to connection secret so crossplane can write to them

Checklist

  • The PR has a meaningful title. It will be used to auto generate the
    changelog.
    The PR has a meaningful description that sums up the change. It will be
    linked in the changelog.
  • PR contains a single logical change (to build a better changelog).
  • Categorize the PR by adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency
    as they show up in the changelog.
  • Link this PR to related issues or PRs.

Copy link
Contributor

@Kidswiss Kidswiss left a comment

Choose a reason for hiding this comment

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

Well that looks easy :D

Just two remarks about naming and the replica ips

component/vshn_postgres.jsonnet Outdated Show resolved Hide resolved
component/vshn_postgres.jsonnet Outdated Show resolved Hide resolved
@@ -403,6 +413,8 @@ local sgCluster = {

comp.FromCompositeFieldPath('spec.parameters.backup.schedule', 'spec.forProvider.manifest.spec.configurations.backups[0].cronSchedule'),
comp.FromCompositeFieldPath('spec.parameters.backup.retention', 'spec.forProvider.manifest.spec.configurations.backups[0].retention'),
comp.FromCompositeFieldPath('spec.parameters.network.serviceType', 'spec.forProvider.manifest.spec.postgresServices.primary.type'),
Copy link
Contributor

Choose a reason for hiding this comment

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

I just saw, that you also have logic in the composition function. Please do these patches there as well. It's confusing if the logic is spread over the composition function and the legacy P+T compostion.

@wejdross wejdross requested a review from Kidswiss August 7, 2023 13:07
@wejdross
Copy link
Member Author

wejdross commented Aug 7, 2023

@Kidswiss please re-review component-appcat as well

@@ -38,6 +38,7 @@ parameters:
services:
vshn:
enabled: true
externalDatabaseConnectionsEnabled: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add this parameter to the default as well? But set it to false, as we can only use it on specific clusters.

Otherwise the compilation will probably fail for the remaining clusters, as there is no field with that name available.

@wejdross wejdross merged commit 7eb5a5c into master Aug 8, 2023
18 checks passed
@wejdross wejdross deleted the add/PSQLServiceType branch August 8, 2023 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants