All fields may not be present in AlertmanagerConfig CRD as its still developing, so if there is a feature request that can be added.

IIUC you want to add source field to PagerDutyConfig? Could you list down if any other fields that are missing and needed?

All fields may not be present in AlertmanagerConfig CRD as its still developing, so if there is a feature request that can be added.

IIUC you want to add source field to PagerDutyConfig? Could you list down if any other fields that are missing and needed?

@slashpai This makes sense, It appears that just source is missing at the moment.

Do you want to contribute to change?

@slashpai i want to solve this issue.

Here is a example PR #5886 how to add fields in alertmanagerconfigs

A general idea:

 Add to https://github.com/prometheus-operator/prometheus-operator/blob/main/pkg/alertmanager/types.go

 Add to https://github.com/prometheus-operator/prometheus-operator/tree/main/pkg/apis/monitoring/v1alpha1 and https://github.com/prometheus-operator/prometheus-operator/tree/main/pkg/apis/monitoring/v1beta1

Run make generate

Add code to add in configs to amcfg.go and tests in amcfg_test.go

Here is a example PR #5886 how to add fields in alertmanagerconfigs

A general idea:

 Add to https://github.com/prometheus-operator/prometheus-operator/blob/main/pkg/alertmanager/types.go

 Add to https://github.com/prometheus-operator/prometheus-operator/tree/main/pkg/apis/monitoring/v1alpha1 and https://github.com/prometheus-operator/prometheus-operator/tree/main/pkg/apis/monitoring/v1beta1

Run make generate

Add code to add in configs to amcfg.go and tests in amcfg_test.go

Thanks for the details, @Ayush9026 I have time to work on this next week if you don't get a chance. Thanks everyone!

Hi @slashpai
I have a few questions

1. In file ./pkg/alertmanager/types.go , @simonpasquier in his commit , already had added the Source config option then why did @Aaron-ML face this issue ?

2. In file ./pkg/alertmanager/amcfg_tests.go in method TestSanitizePagerDutyConfig we are already testing for source to be dropped in case of unsupported versions

{
    name: "Test source is dropped in pagerduty config for unsupported versions",
    againstVersion: semver.Version{Major: 0, Minor: 24},
    in: &alertmanagerConfig{
        Receivers: []*receiver{
            {
                PagerdutyConfigs: []*pagerdutyConfig{
                    {
                        Source: "foo",
                    },
                },
            },
        },
    },
    expect: alertmanagerConfig{
        Receivers: []*receiver{
            {
                PagerdutyConfigs: []*pagerdutyConfig{
                    {
                        Source: "",
                    },
                },
            },
        },
    }
}

I am not sure then if I needed to add any more tests about the Source field as it is an optional template string field. What do you think ?

I am just trying to understand the project better as I want to contribute to kubernetes community. Would really help if you can take out time to answer these questions.

Thanks a lot.

Whenever Alertmanager adds a new option from operator's side 2 things we need to do:

1. support the option in the generated configuration
2. support the option in the AlertmanagerConfig CRD.

This was supported in static config thats why he said in issue it worked there but this option was not supported in AlertmanagerConfig CRD which is a kubernetes native way to define alertmanager config.

Adding to what JP said, the types.go file you are referring to is not really part of the CRD generation. I am not sure, but I think it might be there for marshalling/unmarshalling purposes (correct me if I am wrong). However, that does not allow the user to specify the Source field in the AlertManager CRD. For that it needs to be part of the API and subsequently code-gen must be done for the same, the required changes which you just made in your PR I assume.

Hi @slashpai , Does it make sense to open a new issue for checking for other fields which are optional but pointer type has not been used for them ? I can work creating a PR for that.