tchughesiv commented on code in PR #311:
URL:
https://github.com/apache/incubator-kie-kogito-serverless-operator/pull/311#discussion_r1427091996
##########
controllers/profiles/common/properties/application_test.go:
##########
@@ -238,3 +182,140 @@ func doTestGenerateMicroprofileServiceCatalogProperty(t
*testing.T, serviceUri s
mpProperty := generateMicroprofileServiceCatalogProperty(serviceUri)
assert.Equal(t, mpProperty, expectedProperty, "expected microprofile
service catalog property for serviceUri: %s, is %s, but the returned value was:
%s", serviceUri, expectedProperty, mpProperty)
}
+
+func Test_appPropertyHandler_WithServicesWithUserOverrides(t *testing.T) {
+ //try to override kogito.service.url and quarkus.http.port
+ userProperties :=
"property1=value1\nproperty2=value2\nquarkus.http.port=9090\nkogito.service.url=http://myUrl.override.com\nquarkus.http.port=9090";
+ ns := "default"
+ workflow := test.GetBaseSonataFlow(ns)
+ workflow.SetAnnotations(map[string]string{metadata.Profile:
string(metadata.DevProfile)})
+ enabled := true
+ platform := test.GetBasePlatform()
+ platform.Namespace = ns
+ platform.Spec = operatorapi.SonataFlowPlatformSpec{
+ Services: operatorapi.ServicesPlatformSpec{
+ DataIndex: &operatorapi.ServiceSpec{
+ Enabled: &enabled,
+ },
+ JobService: &operatorapi.ServiceSpec{
+ Enabled: &enabled,
+ },
+ },
+ }
+
+ di := services.NewDataIndexService(platform)
+
+ props, err := NewAppPropertyHandler(workflow, platform)
+ assert.NoError(t, err)
+ generatedProps, propsErr :=
properties.LoadString(props.WithUserProperties(userProperties).Build())
+ assert.NoError(t, propsErr)
+ assert.Equal(t, 8, len(generatedProps.Keys()))
+ assert.Equal(t, "value1", generatedProps.GetString("property1", ""))
+ assert.Equal(t, "value2", generatedProps.GetString("property2", ""))
+ //kogito.service.url takes the user provided value since it's a default
mutable property.
+ assert.Equal(t, "http://myUrl.override.com";,
generatedProps.GetString("kogito.service.url", ""))
+ //quarkus.http.port remains with the default value since it's immutable.
+ assert.Equal(t, "8080", generatedProps.GetString("quarkus.http.port",
""))
+ assert.Equal(t, "0.0.0.0",
generatedProps.GetString("quarkus.http.host", ""))
+ assert.Equal(t, "false",
generatedProps.GetString("org.kie.kogito.addons.knative.eventing.health-enabled",
""))
+ assert.Equal(t, "false",
generatedProps.GetString("quarkus.devservices.enabled", ""))
+ assert.Equal(t, "false",
generatedProps.GetString("quarkus.kogito.devservices.enabled", ""))
+ assert.Equal(t, "",
generatedProps.GetString(constants.DataIndexServiceURLProperty, ""))
+ assert.Equal(t, "",
generatedProps.GetString(constants.JobServiceURLProperty, ""))
+
+ // prod profile enables config of outgoing events url
+ workflow.SetAnnotations(map[string]string{metadata.Profile:
string(metadata.ProdProfile)})
+ props, err = NewAppPropertyHandler(workflow, platform)
+ assert.NoError(t, err)
+ generatedProps, propsErr =
properties.LoadString(props.WithUserProperties(userProperties).Build())
+ assert.NoError(t, propsErr)
+ assert.Equal(t, 13, len(generatedProps.Keys()))
+ assert.Equal(t,
"http://"+platform.Name+"-"+constants.DataIndexServiceName+"."+platform.Namespace+"/processes";,
generatedProps.GetString(constants.DataIndexServiceURLProperty, ""))
+ assert.Equal(t,
"http://"+platform.Name+"-"+constants.JobServiceName+"."+platform.Namespace+"/v2/jobs/events";,
generatedProps.GetString(constants.JobServiceURLProperty, ""))
+ assert.Equal(t, "false",
generatedProps.GetString(constants.JobServiceKafkaSinkInjectionHealthCheck, ""))
+ assert.Equal(t, "",
generatedProps.GetString(constants.JobServiceDataSourceReactiveURLProperty, ""))
+ assert.Equal(t, "true",
generatedProps.GetString(constants.JobServiceStatusChangeEventsProperty, ""))
+ assert.Equal(t,
generatedProps.GetString(constants.JobServiceStatusChangeEventsURL, ""),
fmt.Sprintf("%s://%s.%s/jobs", constants.DataIndexServiceURLProtocol,
di.GetServiceName(), platform.Namespace))
+
+ // disabling data index bypasses config of outgoing events url
+ platform.Spec.Services.DataIndex.Enabled = nil
+ props, err = NewAppPropertyHandler(workflow, platform)
+ assert.NoError(t, err)
+ generatedProps, propsErr =
properties.LoadString(props.WithUserProperties(userProperties).Build())
+ assert.NoError(t, propsErr)
+ assert.Equal(t, 10, len(generatedProps.Keys()))
+ assert.Equal(t, "",
generatedProps.GetString(constants.DataIndexServiceURLProperty, ""))
+ assert.Equal(t,
"http://"+platform.Name+"-"+constants.JobServiceName+"."+platform.Namespace+"/v2/jobs/events";,
generatedProps.GetString(constants.JobServiceURLProperty, ""))
+ assert.Equal(t, "false",
generatedProps.GetString(constants.JobServiceKafkaSinkInjectionHealthCheck, ""))
+ assert.Equal(t, "",
generatedProps.GetString(constants.JobServiceStatusChangeEventsProperty, ""))
+ assert.Equal(t, "",
generatedProps.GetString(constants.JobServiceStatusChangeEventsURL, ""))
+
+ // disabling job service bypasses config of outgoing events url
+ platform.Spec.Services.JobService.Enabled = nil
+ props, err = NewAppPropertyHandler(workflow, platform)
+ assert.NoError(t, err)
+ generatedProps, propsErr =
properties.LoadString(props.WithUserProperties(userProperties).Build())
+ assert.NoError(t, propsErr)
+ assert.Equal(t, 8, len(generatedProps.Keys()))
+ assert.Equal(t, "",
generatedProps.GetString(constants.DataIndexServiceURLProperty, ""))
+ assert.Equal(t, "",
generatedProps.GetString(constants.JobServiceURLProperty, ""))
+ assert.Equal(t, "",
generatedProps.GetString(constants.JobServiceKafkaSinkInjectionHealthCheck, ""))
+ assert.Equal(t, "",
generatedProps.GetString(constants.JobServiceDataSourceReactiveURLProperty, ""))
+ assert.Equal(t, "",
generatedProps.GetString(constants.JobServiceStatusChangeEventsProperty, ""))
+ assert.Equal(t, "",
generatedProps.GetString(constants.JobServiceStatusChangeEventsURL, ""))
+
+ // check that service app properties are being properly set
+ generatedProps, propsErr =
properties.LoadString(NewServiceAppPropertyHandler(platform).WithUserProperties(userProperties).Build())
Review Comment:
> there are no DI or JS properties that requires user intervention
while there may not be properties that "require" user intervention... in my
opinion, the operator shouldn't be in the business of limiting the user wrt
property configs either. we already allow the user to modify just about
everything in the DI & JS PodSpec... application properties should be no
different. this kind of functionality allows for greater adoption and success
in future production-like envs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
