Skip to content
This repository has been archived by the owner on Aug 19, 2024. It is now read-only.

Implement routing configuration for Backstage server endpoint #53

Closed
wants to merge 1 commit into from

Conversation

jianrongzhang89
Copy link
Contributor

@jianrongzhang89 jianrongzhang89 commented Nov 30, 2023

Deploy an OpenShift route on OpenShift clusters.

Description

Changes made:

  1. Autodetect if the current cluster is OpenShift and creates a route when Backstage CR is reconciled.
  2. Changed backstage deployment and service name to "backstage-"+ to allow the users to deploy multiple backstage CRs in the same namespace.
  3. Changed the default value of own-runtien to true.

Note #1: supporting host and tls in the route via Backstage CR is not in the scope of this PR. Such work shall be done later as a separate PR after the data model has been finalized.

Note #2: unit test with isOpenShift=true is not included as OpenShift specific objects (routes, projects etc) are not currently supported by the envTest tool (see operator-framework/operator-sdk#4434).

Which issue(s) does this PR fix or relate to

PR acceptance criteria

  • Tests
  • Documentation

How to test changes / Special notes to the reviewer

Deploy the operator and backstage sample CR on an OpnenShift cluster and check if the OpenShift route for Backstage has been created.

@jianrongzhang89 jianrongzhang89 changed the title Implement routing configuration for Backstage server endpoint [WIP]Implement routing configuration for Backstage server endpoint Nov 30, 2023
@jianrongzhang89 jianrongzhang89 changed the title [WIP]Implement routing configuration for Backstage server endpoint Implement routing configuration for Backstage server endpoint Nov 30, 2023
@jianrongzhang89 jianrongzhang89 changed the title Implement routing configuration for Backstage server endpoint [WIP] Implement routing configuration for Backstage server endpoint Nov 30, 2023
@jianrongzhang89 jianrongzhang89 changed the title [WIP] Implement routing configuration for Backstage server endpoint [WIP-do not review] Implement routing configuration for Backstage server endpoint Nov 30, 2023
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.11.3
Copy link
Member

Choose a reason for hiding this comment

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

Why it is downgraded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected

@@ -187,7 +187,7 @@ var _ = Describe("Backstage controller", func() {
found := &appsv1.Deployment{}
Eventually(func() error {
// TODO to get name from default
return k8sClient.Get(ctx, types.NamespacedName{Namespace: ns, Name: "backstage"}, found)
return k8sClient.Get(ctx, types.NamespacedName{Namespace: ns, Name: "backstage-" + backstageName}, found)
Copy link
Member

Choose a reason for hiding this comment

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

Could you use Sprintf as we usually do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good :)

@jianrongzhang89 jianrongzhang89 force-pushed the route branch 2 times, most recently from 33cb07a to 383c941 Compare December 5, 2023 19:09
@jianrongzhang89 jianrongzhang89 changed the title [WIP-do not review] Implement routing configuration for Backstage server endpoint Implement routing configuration for Backstage server endpoint Dec 5, 2023
@jianrongzhang89 jianrongzhang89 force-pushed the route branch 2 times, most recently from 1781469 to b0d0381 Compare December 7, 2023 16:20
Copy link
Member

@rm3l rm3l left a comment

Choose a reason for hiding this comment

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

Also, can you rebase your branch to resolve the conflicts reported?

@@ -129,6 +125,14 @@ func (r *BackstageReconciler) addBackendAuthEnvVar(ctx context.Context, backstag
Name: "APP_CONFIG_backend_auth_keys",
Value: `[{"secret": "$(BACKEND_SECRET)"}]`,
})
// If a local PostGres DB is used, set POSTGRES_HOST env variable to the local PostGres DB service.
if !backstage.Spec.SkipLocalDb {
Copy link
Member

Choose a reason for hiding this comment

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

This file is more about handling the Backstage Backend Auth Key, not setting info about the local DB.
For better separation of concerns (and also to avoid Git conflicts since this file has changed a bit in #56 - see https://github.com/janus-idp/operator/pull/56/files#diff-d608a9323a5923cb475353e2f8d44c7571b93e49a4fd40f69ab7d1fcbef1801a), I'd suggest adding this env var elsewhere.. Maybe in BackstageReconciler.addEnvVars in backstage_controller.go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. Moved to BackstageReconciler.addEnvVars.

@jianrongzhang89
Copy link
Contributor Author

Close and resubmit as another PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Specify and implement routing configuration for Backstage server endpoint
3 participants