Skip to content

Commit 6f6edec

Browse files
maciej-lechbpg
maciej-lech
and
bpg
authored
fix(apt): add support for modern APT sources (PVE9+) (#2376)
* fix(apt): add support for modern APT sources (PVE9+) Signed-off-by: Maciej Lech <maciej.lech@mlit.pro> * address gemini code review comments Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> --------- Signed-off-by: Maciej Lech <maciej.lech@mlit.pro> Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> Co-authored-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com>
1 parent 2f7bbb1 commit 6f6edec

File tree

8 files changed

+354
-52
lines changed

8 files changed

+354
-52
lines changed

‎fwprovider/nodes/apt/datasource_standard_repo.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ func (d *standardRepositoryDataSource) Read(
8686
return
8787
}
8888

89-
srp.importFromAPI(ctx, data)
89+
ver := getProxmoxVersion(ctx, d.client)
90+
srp.importFromAPI(ctx, data, ver)
9091

9192
resp.Diagnostics.Append(resp.State.Set(ctx, &srp)...)
9293
}

‎fwprovider/nodes/apt/models.go‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ import (
1717
"github.com/hashicorp/terraform-plugin-framework/types"
1818

1919
customtypes "github.com/bpg/terraform-provider-proxmox/fwprovider/types/nodes/apt"
20+
"github.com/hashicorp/terraform-plugin-log/tflog"
21+
22+
"github.com/bpg/terraform-provider-proxmox/proxmox"
2023
api "github.com/bpg/terraform-provider-proxmox/proxmox/nodes/apt/repositories"
24+
"github.com/bpg/terraform-provider-proxmox/proxmox/version"
2125
)
2226

2327
// Note that most constants are exported to allow the usage in (acceptance) tests.
@@ -219,7 +223,7 @@ func (rp *modelRepo) importFromAPI(ctx context.Context, data *api.GetResponseDat
219223
}
220224

221225
// importFromAPI imports the contents of an APT standard repository from the Proxmox VE API's response data.
222-
func (srp *modelStandardRepo) importFromAPI(_ context.Context, data *api.GetResponseData) {
226+
func (srp *modelStandardRepo) importFromAPI(_ context.Context, data *api.GetResponseData, proxmoxVersion *version.ProxmoxVersion) {
223227
for _, repo := range data.StandardRepos {
224228
if repo.Handle == srp.Handle.ValueString() {
225229
srp.Description = types.StringPointerValue(repo.Description)
@@ -240,24 +244,24 @@ func (srp *modelStandardRepo) importFromAPI(_ context.Context, data *api.GetResp
240244
}
241245

242246
// Set the index…
243-
srp.setIndex(data)
247+
srp.setIndex(data, proxmoxVersion)
244248
// … and then the file path when the index is valid…
245249
if !srp.Index.IsNull() {
246250
// …by iterating through all repository files…
247251
for _, repoFile := range data.Files {
248252
// …and get the repository when the file path matches.
249-
if srp.Handle.IsSupportedFilePath(repoFile.Path) {
253+
if srp.Handle.IsSupportedFilePath(repoFile.Path, proxmoxVersion) {
250254
srp.FilePath = types.StringValue(repoFile.Path)
251255
}
252256
}
253257
}
254258
}
255259

256260
// setIndex sets the index of the APT standard repository derived from the defining source list file.
257-
func (srp *modelStandardRepo) setIndex(data *api.GetResponseData) {
261+
func (srp *modelStandardRepo) setIndex(data *api.GetResponseData, proxmoxVersion *version.ProxmoxVersion) {
258262
for _, file := range data.Files {
259263
for idx, repo := range file.Repositories {
260-
if slices.Contains(repo.Components, srp.Handle.ComponentName()) {
264+
if slices.Contains(repo.Components, srp.Handle.ComponentName(proxmoxVersion)) {
261265
// Return early for non-Ceph repositories…
262266
if !srp.Handle.IsCephHandle() {
263267
srp.Index = types.Int64Value(int64(idx))
@@ -279,3 +283,19 @@ func (srp *modelStandardRepo) setIndex(data *api.GetResponseData) {
279283

280284
srp.Index = types.Int64Null()
281285
}
286+
287+
// getProxmoxVersion retrieves the Proxmox VE version from the API client, falling back to the minimum supported version
288+
// if the API call fails.
289+
func getProxmoxVersion(ctx context.Context, client proxmox.Client) *version.ProxmoxVersion {
290+
ver := version.MinimumProxmoxVersion
291+
if versionResp, err := client.Version().Version(ctx); err == nil {
292+
ver = versionResp.Version
293+
} else {
294+
tflog.Warn(ctx, "Failed to determine Proxmox VE version, assuming minimum supported version.", map[string]any{
295+
"error": err,
296+
"assumed_version": ver.String(),
297+
})
298+
}
299+
300+
return &ver
301+
}

‎fwprovider/nodes/apt/repo_test.go‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222

2323
"github.com/bpg/terraform-provider-proxmox/fwprovider/nodes/apt"
2424
"github.com/bpg/terraform-provider-proxmox/fwprovider/test"
25-
apitypes "github.com/bpg/terraform-provider-proxmox/proxmox/types/nodes/apt/repositories"
2625
)
2726

2827
// Note that some "hard-coded" values must be used because of the way how the Proxmox VE API for APT repositories works.
@@ -51,7 +50,7 @@ func TestAccDataSourceRepo(t *testing.T) {
5150
{
5251
Config: te.RenderConfig(`
5352
data "proxmox_virtual_environment_apt_repository" "test" {
54-
file_path = "/etc/apt/sources.list"
53+
file_path = "/etc/apt/sources.list.d/proxmox.sources"
5554
index = 0
5655
node = "{{.NodeName}}"
5756
}`),
@@ -66,7 +65,7 @@ func TestAccDataSourceRepo(t *testing.T) {
6665
resource.TestCheckResourceAttr(
6766
"data.proxmox_virtual_environment_apt_repository.test",
6867
apt.SchemaAttrNameTerraformID,
69-
"apt_repository_"+strings.ToLower(te.NodeName)+"_etc_apt_sources_list_0",
68+
"apt_repository_"+strings.ToLower(te.NodeName)+"_etc_apt_sources_list_d_proxmox_sources_0",
7069
),
7170
test.ResourceAttributesSet("data.proxmox_virtual_environment_apt_repository.test", []string{
7271
"components.#",
@@ -180,7 +179,7 @@ func TestAccResourceRepoValidInput(t *testing.T) {
180179
Config: te.RenderConfig(`
181180
resource "proxmox_virtual_environment_apt_repository" "test" {
182181
enabled = true
183-
file_path = "/etc/apt/sources.list"
182+
file_path = "/etc/apt/sources.list.d/debian.sources"
184183
index = 0
185184
node = "{{.NodeName}}"
186185
}`),
@@ -213,11 +212,11 @@ func TestAccResourceRepoValidInput(t *testing.T) {
213212
map[int]knownvalue.Check{
214213
// The possible Debian version is based on the official table of the Proxmox VE FAQ page:
215214
// - https://pve.proxmox.com/wiki/FAQ#faq-support-table
216-
// - https://www.thomas-krenn.com/en/wiki/Proxmox_VE#Proxmox_VE_8.x
215+
// - https://www.thomas-krenn.com/en/wiki/Proxmox_VE#Proxmox_VE_9.x
217216
//
218217
// The required Proxmox VE version for this provider is of course also taken into account:
219218
// - https://github.com/bpg/terraform-provider-proxmox?tab=readme-ov-file#requirements
220-
0: knownvalue.StringRegexp(regexp.MustCompile(`(bookworm)`)),
219+
0: knownvalue.StringRegexp(regexp.MustCompile(`(trixie)`)),
221220
},
222221
),
223222
),
@@ -226,7 +225,7 @@ func TestAccResourceRepoValidInput(t *testing.T) {
226225
tfjsonpath.New(apt.SchemaAttrNameURIs),
227226
knownvalue.ListPartial(
228227
map[int]knownvalue.Check{
229-
0: knownvalue.StringRegexp(regexp.MustCompile(`https?://ftp\.([a-z]+\.)?debian\.org/debian`)),
228+
0: knownvalue.StringRegexp(regexp.MustCompile(`https?://([a-z]+\.)?debian\.org/debian/`)),
230229
},
231230
),
232231
),
@@ -235,14 +234,14 @@ func TestAccResourceRepoValidInput(t *testing.T) {
235234
Check: resource.ComposeTestCheckFunc(
236235
test.ResourceAttributes("proxmox_virtual_environment_apt_repository.test", map[string]string{
237236
"enabled": strconv.FormatBool(true),
238-
"file_path": "/etc/apt/sources.list",
237+
"file_path": "/etc/apt/sources.list.d/debian.sources",
239238
"index": strconv.FormatInt(0, 10),
240239
"node": te.NodeName,
241240
"id": fmt.Sprintf(
242241
"apt_repository_%s_%s_%d",
243242
strings.ToLower(te.NodeName),
244243
apt.RepoIDCharReplaceRegEx.ReplaceAllString(
245-
strings.TrimPrefix("/etc/apt/sources.list", "/"),
244+
strings.TrimPrefix("/etc/apt/sources.list.d/debian.sources", "/"),
246245
"_",
247246
),
248247
0,
@@ -259,7 +258,7 @@ func TestAccResourceRepoValidInput(t *testing.T) {
259258
ImportStateId: fmt.Sprintf(
260259
"%s,%s,%d",
261260
strings.ToLower(te.NodeName),
262-
apitypes.StandardRepoFilePathMain,
261+
"/etc/apt/sources.list.d/debian.sources",
263262
testAccResourceRepoIndex,
264263
),
265264
ImportStateVerify: true,
@@ -271,7 +270,7 @@ func TestAccResourceRepoValidInput(t *testing.T) {
271270
Config: te.RenderConfig(`
272271
resource "proxmox_virtual_environment_apt_repository" "test" {
273272
enabled = false
274-
file_path = "/etc/apt/sources.list"
273+
file_path = "/etc/apt/sources.list.d/debian.sources"
275274
index = 0
276275
node = "{{.NodeName}}"
277276
}`),
@@ -309,7 +308,7 @@ func TestAccResourceStandardRepoValidInput(t *testing.T) {
309308
// Test the "Create" and "Read" implementations.
310309
{
311310
// PUT /api2/json/nodes/{node}/apt/repositories with handle = "no-subscription" will create a new
312-
// entry in /etc/apt/sources.list on each call :/
311+
// entry in /etc/apt/sources.list.d/proxmox.sources on each call :/
313312
SkipFunc: func() (bool, error) {
314313
return true, nil
315314
},
@@ -321,7 +320,7 @@ func TestAccResourceStandardRepoValidInput(t *testing.T) {
321320
// The provided attributes and computed attributes should be set.
322321
Check: resource.ComposeTestCheckFunc(
323322
test.ResourceAttributes("proxmox_virtual_environment_apt_standard_repository.test", map[string]string{
324-
"file_path": "/etc/apt/sources.list",
323+
"file_path": "/etc/apt/sources.list.d/proxmox.sources",
325324
"handle": "no-subscription",
326325
"node": te.NodeName,
327326
"status": "1",

‎fwprovider/nodes/apt/resource_standard_repo.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ func (r *standardRepositoryResource) read(ctx context.Context, srp *modelStandar
6868
}
6969
}
7070

71-
srp.importFromAPI(ctx, data)
71+
ver := getProxmoxVersion(ctx, r.client)
72+
srp.importFromAPI(ctx, data, ver)
7273

7374
return true, nil
7475
}

‎fwprovider/types/nodes/apt/standard_repo_handle.go‎

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/hashicorp/terraform-plugin-go/tftypes"
1919

2020
apitypes "github.com/bpg/terraform-provider-proxmox/proxmox/types/nodes/apt/repositories"
21+
"github.com/bpg/terraform-provider-proxmox/proxmox/version"
2122
)
2223

2324
// Ensure the implementations satisfy the required interfaces.
@@ -139,10 +140,10 @@ func (v StandardRepoHandleValue) CephVersionName() apitypes.CephVersionName {
139140
}
140141

141142
// ComponentName returns the corresponding component name.
142-
func (v StandardRepoHandleValue) ComponentName() string {
143+
func (v StandardRepoHandleValue) ComponentName(proxmoxVersion *version.ProxmoxVersion) string {
143144
if v.cvn == apitypes.CephVersionNameUnknown && v.kind != apitypes.StandardRepoHandleKindUnknown {
144-
// For whatever reason the non-Ceph handle "test" kind does not use a dash in between the "pve" prefix.
145-
if v.kind == apitypes.StandardRepoHandleKindTest {
145+
// On PVE8, for whatever reason the non-Ceph handle "test" kind does not use a dash in between the "pve" prefix.
146+
if proxmoxVersion != nil && !proxmoxVersion.SupportModernAptSources() && v.kind == apitypes.StandardRepoHandleKindTest {
146147
return fmt.Sprintf("pve%s", v.kind)
147148
}
148149

@@ -169,17 +170,37 @@ func (v StandardRepoHandleValue) IsCephHandle() bool {
169170
}
170171

171172
// IsSupportedFilePath returns whether the handle is supported for the given source list file path.
172-
func (v StandardRepoHandleValue) IsSupportedFilePath(filePath string) bool {
173-
switch filePath {
174-
case apitypes.StandardRepoFilePathCeph:
175-
return v.IsCephHandle()
176-
case apitypes.StandardRepoFilePathEnterprise:
177-
return !v.IsCephHandle() && v.kind == apitypes.StandardRepoHandleKindEnterprise
178-
case apitypes.StandardRepoFilePathMain:
179-
return !v.IsCephHandle() && v.kind != apitypes.StandardRepoHandleKindEnterprise
180-
default:
181-
return false
173+
// The proxmoxVersion parameter is used to determine whether to use old (.list) or new (.sources) format paths.
174+
// For versions below PVE 9: only old .list paths are supported.
175+
// For PVE 9 and above: both old and new paths are supported for compatibility.
176+
func (v StandardRepoHandleValue) IsSupportedFilePath(filePath string, proxmoxVersion *version.ProxmoxVersion) bool {
177+
supportsModernPaths := proxmoxVersion == nil || proxmoxVersion.SupportModernAptSources()
178+
179+
if v.IsCephHandle() {
180+
if supportsModernPaths {
181+
return filePath == apitypes.StandardRepoFilePathCeph || filePath == apitypes.OldStandardRepoFilePathCeph
182+
}
183+
184+
return filePath == apitypes.OldStandardRepoFilePathCeph
185+
}
186+
187+
if v.kind == apitypes.StandardRepoHandleKindEnterprise {
188+
if supportsModernPaths {
189+
return filePath == apitypes.StandardRepoFilePathEnterprise || filePath == apitypes.OldStandardRepoFilePathEnterprise
190+
}
191+
192+
return filePath == apitypes.OldStandardRepoFilePathEnterprise
193+
}
194+
195+
if v.kind == apitypes.StandardRepoHandleKindNoSubscription || v.kind == apitypes.StandardRepoHandleKindTest {
196+
if supportsModernPaths {
197+
return filePath == apitypes.StandardRepoFilePathMain || filePath == apitypes.OldStandardRepoFilePathMain
198+
}
199+
200+
return filePath == apitypes.OldStandardRepoFilePathMain
182201
}
202+
203+
return false
183204
}
184205

185206
// Type returns the type of the value.

0 commit comments

Comments
 (0)